[openstack-dev] [puppet] Change abandonment policy

Colleen Murphy colleen at gazlene.net
Tue Jun 2 18:39:39 UTC 2015


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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150602/e1637009/attachment.html>


More information about the OpenStack-dev mailing list