Open Stack

+1 for #3 with N = 1

regards
/sanjay

On Fri, Jun 5, 2015 at 1:27 AM, Mike Dorman <mdorman at godaddy.com> wrote:

>   I vote #2, with a smaller N.
>
>  We can always adjust this policy in the future if find we have to
> manually abandon too many old reviews.
>
>
>   From: Colleen Murphy
> Reply-To: "puppet-openstack at puppetlabs.com"
> Date: Tuesday, June 2, 2015 at 12:39 PM
> To: "OpenStack Development Mailing List (not for usage questions)", "
> puppet-openstack at puppetlabs.com"
> Subject: [puppet] Change abandonment policy
>
>   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.
>
>  Colleen
>
>  [1]
> https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh
>
> --
>
>  To unsubscribe from this group and stop receiving emails from it, send an
> email to puppet-openstack+unsubscribe at puppetlabs.com.
>
> --
>
>  To unsubscribe from this group and stop receiving emails from it, send an
> email to puppet-openstack+unsubscribe at puppetlabs.com.
>



-- 
Sanjay Upadhyay
http://saneax.blogspot.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150605/0de0278d/attachment.html>