[Spice-devel] SPICE: changing the merge rules - a proposal

[Jeremy White]

Hi,

> 
>     The x11spice project is another example: we have only 4 contributors
>     from the beginning of the year (and a single contributor holding 70% of
>     the commits) and the reviews on the gitlab merge requests have been
>     provided by two people only, each one reviewing the merge requests of
>     the other.
> 
> 
> As projects become more specific/marginal, it's clear that the number of 
> maintainers is lower. Yet, we have those projects under the same 
> umbrella, and I don't think they should be treated differently. 2 active 
> developers/maintainers can be very healthy, I would say. So do we have a 
> maintenance issue with x11spice? Do we want to move the project out of 
> the "Spice-space" projects for ex? What is the problem exactly?

The problem is that any merge request I put in sits there until Frediano 
comes around to look at it.  I do have commit rights, but I have not 
felt comfortable exercising those unilaterally.  I am a fan of the core 
Spice policy - commits only after review.  And I have had patches and 
MRs sit for very long periods of time with no review.  Frediano is right 
that the websocket change is a good example - it is a material 
improvement for anyone using the spice-html5 client, and it lingered for 
years without going in.

There were certainly patch sets that were small enough that I think it 
would have been reasonable for me to just 'time out' and apply them 
without review.  Perhaps I'm not using the 'trivial' policy as I should, 
and I think Daniel is right that there just aren't enough of us.

But another truth is I could have shouted louder and more often and 
perhaps gotten the websocket patch in faster, but it wasn't impacting me 
or any of my customers, so I didn't.

> 
>     For the sake of having the projects being able to move forward with a
>     reduced number of contributors/reviewers, the proposal is to *allow* a
>     maintainer to merge a Merge Request without an explicit ack if the
>     three
>     following conditions are met:
>     1) The Merge Request has been pending for at least 3 weeks without
>     getting new comments
>     2) The Merge Request submitter has kept asking a review on a weekly
>     basis
>     3) There are no pending nacks on the Merge Request
> 
> 
> It's hard to define a delay to bypass a reviewing & consensus rule. In 
> general, it should really be frowned upon. There is clearly more than 
> one person interested and using Spice. If the issue is important, it 
> should be fairly easy to get someone else to look at the issue in a 
> timely manner. If not, there are probably more important/interesting 
> things to do to get other interested.

I have to say that I was really startled to find that the C++ change had 
been merged.  I largely don't review spice server patches, as I consider 
myself a consumer of the spice server, and not expert enough to comment 
on most patches.  To my discredit, I haven't really adapted to the 
gitlab era; I have not subscribed to MRs in areas outside my projects 
(x11spice, spice-html5).

I also agree with Marc-André that this conversation should have taken 
place over the mailing list.  A merge request is not the appropriate 
place to discuss that substantive a change.

And I *would* have noticed a discussion of that significant a change on 
the mailing list, and I might have even stirred myself to invest in it 
enough to give a considered opinion.

Cheers,

Jeremy