On Tue, Nov 18, 2014 at 6:17 AM, Daniel P. Berrange <berra...@redhat.com> wrote:
> 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. > ++ for bringing back auto abandon in this model. > > 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 >
_______________________________________________ OpenStack-dev mailing list OpenStack-dev@lists.openstack.org http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
