Re: SPICE: changing the merge rules - a proposal

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux Virtualization]     [Linux Virtualization]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]