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

Daniel P. Berrange berrange at redhat.com
Tue Nov 18 12:29:04 UTC 2014


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.



Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|



More information about the OpenStack-dev mailing list