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

2020-05-19 Thread Marc-André Lureau
Hi

On Tue, May 19, 2020 at 11:10 AM Frediano Ziglio  wrote:

> >
> > Hi,
> >the community around the SPICE project always tried to follow one
> > fundamental, implicit rule for accepting code contributions to the
> > project: every merge request (beside trivial patches) should be reviewed
> > and acked at least by one before getting merged.
> > While everyone agrees with this fundamental rule, the actual status of
> > some SPICE projects makes the rule impractical to let the project move
> > forward.
> > Let's consider the spice/spice project as an example: the number of
> > contributions is very low, both on the commit side (only 4 different
> > contributors with more than 1 commit from the beginning of the year, and
> > a single contributor with 90% of commits) and on the review side (in the
> > last 40 merge requests before the C++ switch one, 21 had no comments).
> > 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.
> > 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
> >
> > Note that having patches reviewed would still be the preferred way. If
> > at any time the number of contributors would raise again, we can switch
> > back to the mandatory review rule. Until then the priority is to allow
> > the project to move forward.
> >
> > What do you think? Please share your thoughts and/or contribute with
> > your own ideas.
> >
> > Thanks
> >
> > Francesco
> >
>
> Hi,
>I agree on the proposal. This problem has been going on for years now.
> The issue involve all repositories. The active part of community dealing
> with code review is very low.
>

All open-source projects are under-staffed. Spice has had a number of paid
developpers, and a number of paid developpers who can help when it is
needed. This is better than a lot of projects.

I think 3 weeks are perfectly fine. We should be in a trustful community,
>

I expressed my anxiousness about that short period. This is not going to be
healthy to put such constrain. If we were unhappy about the time it took to
review a MR, we would need first to prioritize our work better, and
reaching out for help. Not rush the changes, please.

if somebody is very busy in daily activity or on holidays whomever remains
> should be in charge. Is not that is a person in a team goes on holiday all
> the work of the team is impeded.
>

I don't think such "blocking" factor exist, because the Spice code base is
large, and the domain as well. But in such exceptional circonstances, we
can easily either find a consensus for an exception, or help each other to
unblock the situation.

thanks

-- 
Marc-André Lureau
___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel


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

2020-05-19 Thread Frediano Ziglio
> 
> Hi,
>the community around the SPICE project always tried to follow one
> fundamental, implicit rule for accepting code contributions to the
> project: every merge request (beside trivial patches) should be reviewed
> and acked at least by one before getting merged.
> While everyone agrees with this fundamental rule, the actual status of
> some SPICE projects makes the rule impractical to let the project move
> forward.
> Let's consider the spice/spice project as an example: the number of
> contributions is very low, both on the commit side (only 4 different
> contributors with more than 1 commit from the beginning of the year, and
> a single contributor with 90% of commits) and on the review side (in the
> last 40 merge requests before the C++ switch one, 21 had no comments).
> 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.
> 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
> 
> Note that having patches reviewed would still be the preferred way. If
> at any time the number of contributors would raise again, we can switch
> back to the mandatory review rule. Until then the priority is to allow
> the project to move forward.
> 
> What do you think? Please share your thoughts and/or contribute with
> your own ideas.
> 
> Thanks
> 
> Francesco
> 

Hi,
   I agree on the proposal. This problem has been going on for years now.
The issue involve all repositories. The active part of community dealing
with code review is very low.
I think 3 weeks are perfectly fine. We should be in a trustful community,
if somebody is very busy in daily activity or on holidays whomever remains
should be in charge. Is not that is a person in a team goes on holiday all
the work of the team is impeded.

Frediano

___
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/spice-devel