<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Nov 18, 2014 at 6:17 AM, Daniel P. Berrange <span dir="ltr"><<a href="mailto:berrange@redhat.com" target="_blank">berrange@redhat.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="HOEnZb"><div class="h5">On Tue, Nov 18, 2014 at 07:33:54AM -0500, Sean Dague wrote:<br>
> On 11/18/2014 07:29 AM, Daniel P. Berrange wrote:<br>
> > On Tue, Nov 18, 2014 at 07:06:59AM -0500, Sean Dague wrote:<br>
> >> Nova currently has 197 patches that have seen no activity in the last 4<br>
> >> weeks (project:openstack/nova age:4weeks status:open).<br>
> >><br>
> >> Of these<br>
> >> * 108 are currently Jenkins -1 (project:openstack/nova age:4weeks<br>
> >> status:open label:Verified<=-1,jenkins)<br>
> >> * 60 are -2 by a core team member (project:openstack/nova age:4weeks<br>
> >> status:open label:Code-Review<=-2)<br>
> >><br>
> >> (note, those 2 groups sometimes overlap)<br>
> >><br>
> >> Regardless, the fact that Nova currently has 792 open reviews, and 1/4<br>
> >> of them seem dead, seems like a cleanup thing we could do.<br>
> >><br>
> >> I'd like to propose that we implement our own auto abandon mechanism<br>
> >> based on reviews that are either held by a -2, or Jenkins -1 after 4<br>
> >> weeks time. I can write a quick script to abandon with a friendly<br>
> >> message about why we are doing it, and to restore it if work is continuing.<br>
> > Yep, purging anything that's older than 4 weeks with negative karma<br>
> > seems like a good idea. It'll make it easier for us to identify those<br>
> > patches which are still "maintained" and target them for review.<br>
> ><br>
> > That said, there's some edge cases - for example I've got some patches<br>
> > up for review that have a -2 on them, becase we're waiting for blueprint<br>
> > approval. IIRC, previously we would post a warning about pending auto-<br>
> > abandon a week before, and thus give the author the chance to add a<br>
> > comment to prevent auto-abandon taking place. It would be neccessary to<br>
> > have this ability to deal with the case where we're just temporarily<br>
> > blocked on other work.<br>
> ><br>
> > Also sometimes when you have a large patch series, you might have some<br>
> > patches later in the series which (temporarily) fail the jenkins jobs.<br>
> > It often isn't worth fixing those failures until you have dealt with<br>
> > review earlier in the patch series. So I think we should not auto-expire<br>
> > patches which are in the middle of a patch series, unless the preceeding<br>
> > patches in the series are to be expired too. Yes this isn't something<br>
> > you can figure out with a single gerrit query - you'd have to query<br>
> > gerrit for patches and then look at the parent change references.<br>
> Or just abandon and let people restore. I think handling the logic /<br>
> policy for the edge cases isn't worth it when the author can very easily<br>
> hit the restore button to get their patch back (and fresh for another 4w).<br>
><br>
> If it was a large patch series, this wouldn't happen anyway, because<br>
> every rebase would make it fresh. 4w is really 4w of nothing changing.<br>
<br>
</div></div>Ok, that makes sense and is workable I reckon.<br></blockquote><div><br></div><div>++ for bringing back auto abandon in this model. </div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div class="HOEnZb"><div class="h5"><br>
Regards,<br>
Daniel<br>
--<br>
|: <a href="http://berrange.com" target="_blank">http://berrange.com</a> -o- <a href="http://www.flickr.com/photos/dberrange/" target="_blank">http://www.flickr.com/photos/dberrange/</a> :|<br>
|: <a href="http://libvirt.org" target="_blank">http://libvirt.org</a> -o- <a href="http://virt-manager.org" target="_blank">http://virt-manager.org</a> :|<br>
|: <a href="http://autobuild.org" target="_blank">http://autobuild.org</a> -o- <a href="http://search.cpan.org/~danberr/" target="_blank">http://search.cpan.org/~danberr/</a> :|<br>
|: <a href="http://entangle-photo.org" target="_blank">http://entangle-photo.org</a> -o- <a href="http://live.gnome.org/gtk-vnc" target="_blank">http://live.gnome.org/gtk-vnc</a> :|<br>
<br>
_______________________________________________<br>
OpenStack-dev mailing list<br>
<a href="mailto:OpenStack-dev@lists.openstack.org">OpenStack-dev@lists.openstack.org</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</div></div></blockquote></div><br></div></div>