Open Stack

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 :|