Open Stack

On 06/02/2015 02:39 PM, Colleen Murphy wrote:
> In today's meeting we discussed implementing a policy for whether and
> when core reviewers should abandon old patches whose author's were
> inactive. (This doesn't apply to authors that want to abandon their own
> changes, only for core reviewers to abandon other people's changes.)
> There are a few things we could do here, with potential policy drafts
> for the wiki:
> 
> 1) Never abandon
> 
> ```
> Our policy is to never abandon changes except for our own.
> ```
> 
> The sentiment here is that an old change in the queue isn't really
> hurting anything by just sitting there, and it is more visible if
> someone else wants to pick up the change.
> 
> 2) Manually abandon after N months/weeks changes that have a -2 or were
> fixed in a different patch
> 
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC
> or email. If the change is easy to fix, anyone should feel welcome to
> check out the change and resubmit it using the same change ID to
> preserve original authorship. Core reviewers will not abandon such a change.
> 
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change
> was chosen to solve the problem), and the author has been unresponsive
> for at least 3 months, a core reviewer should abandon the change.
> ```
> 
> Core reviewers can click the abandon button only on old patches that are
> definitely never going to make it in. This approach has the advantage
> that it is easier for contributors to find changes and fix them up, even
> if the change is very old.
> 
> 3) Manually abandon after N months/weeks changes that have a -1 that was
> never responded to
> 
> ```
> If a change is submitted and given a -1, and subsequently the author
> becomes unresponsive for a few weeks, reviewers should leave reminder
> comments on the review or attempt to contact the original author via IRC
> or email. If the change is easy to fix, anyone should feel welcome to
> check out the change and resubmit it using the same change ID to
> preserve original authorship. If the author is unresponsive for at least
> 3 months and no one else takes over the patch, core reviewers can
> abandon the patch, leaving a detailed note about how the change can be
> restored.
> 
> If a change is submitted and given a -2, or it otherwise becomes clear
> that the change can not make it in (for example, if an alternate change
> was chosen to solve the problem), and the author has been unresponsive
> for at least 3 months, a core reviewer should abandon the change.
> ```
> 
> Core reviewers can click the abandon button on changes that no one has
> shown an interest in in N months/weeks, leaving a message about how to
> restore the change if the author wants to come back to it. Puppet Labs
> does this for its module pull requests, setting N at 1 month.
> 
> 4) Auto-abandon after N months/weeks if patch has a -1 or -2
> 
> ```
> If a change is given a -2 and the author has been unresponsive for at
> least 3 months, a script will automatically abandon the change, leaving
> a message about how the author can restore the change and attempt to
> resolve the -2 with the reviewer who left it.
> ```
> 
> We would use a tool like this one[1] to automatically abandon changes
> meeting a certain criteria. We would have to decide whether we want to
> only auto-abandon changes with -2's or go as far as to auto-abandon
> those with -1's. The policy proposal above assumes -2. The tool would
> leave a canned message about how to restore the change.
> 
> 
> Option 1 has the problem of leaving clutter around, which the discussion
> today seeks to solve.
> 
> Option 3 leaves the possibility that a change that is mostly good
> becomes abandoned, making it harder for someone to find and restore it.
> 
>  I don't think option 4 is necessary because there are not an
> overwhelming number of old changes (I count 9 that are currently over
> six months old). In working through old changes a few months ago I found
> that many of them are easy to fix up to remove a -1, and auto-abandoning
> removes the ability for a human to make that call. Moreover, if a patch
> has a procedural -2 that ought to be lifted after some point,
> auto-abandonment has the potential to accidentally throw out a change
> that was intended to be kept (though presumably the core reviewer who
> left the -2 would notice the abandonment and restore it if that was the
> case). 
> 
> I am in favor of option 2. I think setting N to be 3 months or 6 months
> is appropriate. I don't have very strong feelings about options 1 or 3.
> I'm against option 4.

#2 is good indeed, it also avoid to discourage new contributors that
need more time to provide a good patch.

Like you said, some OpenStack projects use #4 because they have tons of
patches. I don't think we will reach this scale one day, but I still
like #4 for big projects.

Big +1 for #2, thanks Colleen for bringing that,

> Colleen
> 
> [1] https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh
> 
> 
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
> 

-- 
Emilien Macchi

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150606/55607bbd/attachment.pgp>