[openstack-dev] [puppet] Change abandonment policy

Andrew Woodward xarses at gmail.com
Tue Jun 2 18:53:27 UTC 2015


Also in favor of #2 and thought it was how it was running. #4 sounds bad
and may hide good code.

How do we want to account for drive-by authors who are going to be unable
to work on future revisions. We talked a while back that we wanted to be
able to account for this as some operators are unable to do/have time for
proper CR cycles to get patches landed.

On Tue, Jun 2, 2015 at 11:39 AM Colleen Murphy <colleen at gazlene.net> 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.
>
> 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.
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150602/db7369b5/attachment.html>


More information about the OpenStack-dev mailing list