On Tue, Nov 18, 2014 at 07:33:54AM -0500, Sean Dague wrote: > 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.
Ok, that makes sense and is workable I reckon. 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 :| _______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
