<div dir="ltr">Also in favor of #2 and thought it was how it was running. #4 sounds bad and may hide good code.<br><br><div>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.</div></div><br><div class="gmail_quote"><div dir="ltr">On Tue, Jun 2, 2015 at 11:39 AM Colleen Murphy <<a href="mailto:colleen@gazlene.net">colleen@gazlene.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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:<div><br></div><div>1) Never abandon</div><div><br></div><div>```</div><div>Our policy is to never abandon changes except for our own.</div><div>```</div><div><br></div><div>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.</div><div><br></div><div>2) Manually abandon after N months/weeks changes that have a -2 or were fixed in a different patch</div><div><br></div><div>```</div><div><div>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.</div><div><br></div><div>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.</div></div><div>```</div><div><br></div><div>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.</div><div><br></div><div>3) Manually abandon after N months/weeks changes that have a -1 that was never responded to</div><div><br></div><div><div>```</div><div>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.</div><div><br></div><div>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.</div><div>```</div></div><div><br></div><div>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.</div><div><br></div><div>4) Auto-abandon after N months/weeks if patch has a -1 or -2</div><div><br></div><div>```</div><div>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.</div><div>```</div><div><br></div><div>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.</div><div><br></div><div><br></div><div>Option 1 has the problem of leaving clutter around, which the discussion today seeks to solve.</div><div><br></div><div>Option 3 leaves the possibility that a change that is mostly good becomes abandoned, making it harder for someone to find and restore it.<br></div><div><br></div><div> 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). <br></div><div><br></div><div>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.</div><div><br></div><div>Colleen</div><div><br></div><div>[1] <a href="https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh" target="_blank">https://github.com/openstack/nova/blob/master/tools/abandon_old_reviews.sh</a></div></div>

<p></p>

-- <br>
<br>

<p></p>

To unsubscribe from this group and stop receiving emails from it, send an email to <a href="mailto:puppet-openstack+unsubscribe@puppetlabs.com" target="_blank">puppet-openstack+unsubscribe@puppetlabs.com</a>.<br>
</blockquote></div>