<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Thu, Aug 28, 2014 at 2:40 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="">On Thu, Aug 28, 2014 at 01:04:57AM +0000, Dugger, Donald D wrote:<br>
> I'll try and not whine about my pet project but I do think there<br>
> is a problem here. For the Gantt project to split out the scheduler<br>
> there is a crucial BP that needs to be implemented (<br>
> <a href="https://review.openstack.org/#/c/89893/" target="_blank">https://review.openstack.org/#/c/89893/</a> ) and, unfortunately, the<br>
> BP has been rejected and we'll have to try again for Kilo. My question<br>
> is did we do something wrong or is the process broken?<br>
><br>
> Note that we originally proposed the BP on 4/23/14, went through 10<br>
> iterations to the final version on 7/25/14 and the final version got<br>
> three +1s and a +2 by 8/5. Unfortunately, even after reaching out<br>
> to specific people, we didn't get the second +2, hence the rejection.<br>
<br>
</div>I see at that it did not even get one +2 at the time of the feature<br>
proposal approval freeze. You then successfully requested an exception<br>
and after a couple more minor updates got a +2 from John but from no<br>
one else.<br>
<br>
I do think this shows a flaw in our (core teams) handling of the<br>
blueprint. When we agreed upon the freeze exception, that should<br>
have included a firm commitment for a least 2 core devs to review<br>
it. IOW I think it is reasonable to say that either your feature<br>
should have ended up with two +2s and +A, or you should have seen<br>
a -1 from another core dev. I don't think it is acceptable that<br>
after the exception was approved it only got feedback from one<br>
core dev. I actually thought that when approving exceptions, we<br>
always got 2 cores to agree to review the item to avoid this, so<br>
I'm not sure why we failed here.<br>
<div class=""><br>
> I understand that reviews are a burden and very hard but it seems<br>
> wrong that a BP with multiple positive reviews and no negative<br>
> reviews is dropped because of what looks like indifference. Given<br>
> that there is still time to review the actual code patches it seems<br>
> like there should be a simpler way to get a BP approved. Without<br>
> an approved BP it's difficult to even start the coding process.<br></div></blockquote><div><br></div><div>So the question is the BP approval process broken doesn't have a simple answer. There are definitely things we should change, but in this case I think the process sort of worked. The problem you hit is we just don't have enough people doing reviews. Your blueprint didn't get approved in part because the ratio of reviews needed to reviewers is off. If we don't even have enough bandwidth to approve this spec we certainly don't have enough bandwidth to review the code associated with the spec.</div>
<div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="">
><br>
> I see 2 possibilities here:<br>
><br>
><br>
> 1) This is an isolated case specific to this BP. If so,<br>
> there's no need to change the procedures but I would like to<br>
> know what we should be doing differently. We got a +2 review<br>
> on 8/4 and then silence for 3 weeks.<br>
><br>
> 2) This is a process problem that other people encounter.<br>
> Maybe there are times when silence means assent. Something<br>
> like a BP with multiple +1s and at least one +2 should<br>
> automatically be accepted if no one reviews it 2 weeks after<br>
> the +2 is given.<br>
<br>
</div>My two thoughts are<br>
<br>
- When we approve something for exception should actively monitor<br>
progress of review to ensure it gets the neccessary attention to<br>
either approve or reject it. It makes no sense to approve an<br>
exception and then let it lie silently waiting for weeks with no<br>
attention. I'd expect that any time exceptions are approved we<br>
should babysit them and actively review their status in the weekly<br>
meeting to ensure they are followed up on.<br>
<br>
- Core reviewers should prioritize reviews of things which already<br>
have a +2 on them. I wrote about this in the context of code reviews<br>
last week, but all my points apply equally to spec reviews I believe.<br>
<br>
<a href="http://lists.openstack.org/pipermail/openstack-dev/2014-August/043657.html" target="_blank">http://lists.openstack.org/pipermail/openstack-dev/2014-August/043657.html</a><br>
<br>
Also note that in Kilo the process will be slightly less heavyweight in<br>
that we're going to try allow some features changes into tree without<br>
first requiring a spec/blueprint to be written. I can't say offhand<br>
whether this particular feature would have qualifying for the lighter<br>
process, but in general by reducing need for specs for the more trivial<br>
items, we'll have more time available for review of things which do<br>
require specs.<br></blockquote><div><br></div><div>Under the proposed changes to the spec/blueprint process, this would still need a spec.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Regards,<br>
Daniel<br>
<span class="HOEnZb"><font color="#888888">--<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>
</font></span></blockquote></div><br></div></div>