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

Joe Gordon joe.gordon0 at gmail.com
Tue Nov 18 22:38:38 UTC 2014


On Tue, Nov 18, 2014 at 6:17 AM, Daniel P. Berrange <berrange at 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 at lists.openstack.org
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20141118/f52b62d7/attachment.html>


More information about the OpenStack-dev mailing list