[openstack-dev] [nova] policy on old / virtually abandoned patches

Sean Dague sean at dague.net
Tue Nov 18 12:33:54 UTC 2014


On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:
> On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:
>> Nova currently has 197 patches that have seen no activity in the last 4
>> weeks (project:openstack/nova age:4weeks status:open).
>>
>> Of these
>>  * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks
>> status:open label:Verified<=-1,jenkins)
>>  * 60 are -2 by a core team member (project:openstack/nova age:4weeks
>> status:open label:Code-Review<=-2)
>>
>> (note, those 2 groups sometimes overlap)
>>
>> Regardless, the fact that Nova currently has 792 open reviews, and 1/4
>> of them seem dead, seems like a cleanup thing we could do.
>>
>> I'd like to propose that we implement our own auto abandon mechanism
>> based on reviews that are either held by a -2, or Jenkins -1 after 4
>> weeks time. I can write a quick script to abandon with a friendly
>> message about why we are doing it, and to restore it if work is continuing.
> Yep, purging anything that's older than 4 weeks with negative karma
> seems like a good idea.  It'll make it easier for us to identify those
> patches which are still "maintained" and target them for review.
>
> That said, there's some edge cases - for example I've got some patches
> up for review that have a -2 on them, becase we're waiting for blueprint
> approval. IIRC, previously we would post a warning about pending auto-
> abandon a week before, and thus give the author the chance to add a
> comment to prevent auto-abandon taking place. It would be neccessary to
> have this ability to deal with the case where we're just temporarily
> blocked on other work.
>
> Also sometimes when you have a large patch series, you might have some
> patches later in the series which (temporarily) fail the jenkins jobs.
> It often isn't worth fixing those failures until you have dealt with
> review earlier in the patch series. So I think we should not auto-expire
> patches which are in the middle of a patch series, unless the preceeding
> patches in the series are to be expired too.  Yes this isn't something
> you can figure out with a single gerrit query - you'd have to query
> gerrit for patches and then look at the parent change references.
Or just abandon and let people restore. I think handling the logic /
policy for the edge cases isn't worth it when the author can very easily
hit the restore button to get their patch back (and fresh for another 4w).

If it was a large patch series, this wouldn't happen anyway, because
every rebase would make it fresh. 4w is really 4w of nothing changing.

    -Sean

-- 
Sean Dague
http://dague.net


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 465 bytes
Desc: OpenPGP digital signature
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141118/03fa3c04/attachment.pgp>


More information about the OpenStack-dev mailing list