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-requ...@lists.openstack.org?subject:unsubscribe > http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev > -- Emilien Macchi
signature.asc
Description: OpenPGP digital signature
__________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev