<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Tue, Aug 19, 2014 at 8:47 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:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div class="">On Tue, Aug 19, 2014 at 09:31:48PM +1200, Robert Collins wrote:<br>


</div><div class="">> Hey everybody - <a href="https://wiki.openstack.org/wiki/TripleO/SpecReviews" target="_blank">https://wiki.openstack.org/wiki/TripleO/SpecReviews</a><br>
> seems pretty sane as we discussed at the last TripleO IRC meeting.<br>
><br>
> I'd like to propose that we adopt it with the following tweak:<br>
><br>
> 19:46:34 <lifeless> so I propose that +2 on a spec is a commitment to<br>
> review it over-and-above the core review responsibilities<br>
> 19:47:05 <lifeless> if its not important enough for a reviewer to do<br>
> that thats a pretty strong signal<br>
> 19:47:06 <dprince> lifeless: +1, I thought we already agreed to that<br>
> at the meetup<br>
> 19:47:17 <slagle> yea, sounds fine to me<br>
> 19:47:20 <bnemec> +1<br>
> 19:47:30 <lifeless> dprince: it wasn't clear whether it was<br>
> part-of-responsibility, or additive, I'm proposing we make it clearly<br>
> additive<br>
> 19:47:52 <lifeless> and separately I think we need to make surfacing<br>
> reviews-for-themes a lot better<br>
><br>
> That is - +1 on a spec review is 'sure, I like it', +2 is specifically<br>
> "I will review this *over and above* my core commitment" - the goal<br>
> here is to have some very gentle choke on concurrent WIP without<br>
> needing the transition to a managed pull workflow that Nova are<br>
> discussing - which we didn't have much support for during the meeting.<br>
<br>
</div>If it is considered to be a firm commitment to review the code, then<br>
people will have to be quite conservative when approving blueprints<br>
lest take accept too much work. It is hard to predict just how much<br>
work will be involved in the code review from the spec though, and<br>
even harder to predict /when/ during the dev cycle the code will be<br>
posted.<br>
<br>
I could approve 10 specs and think that's reasonable for the dev<br>
cycle, but if those 10 all have their code posted in Kilo-3, then<br>
I could be sitting idle with no features to review during Kilo-1/2<br>
- time that could have been filled with specs I chose not to approve.<br>
And similarly I can still end up with far too much work during the<br>
Kilo3 phase to review.<br></blockquote><div><br></div><div>From the wiki:</div><div><br></div><div><ul style="padding:0px;margin:0.3em 0px 0px 1.6em;color:rgb(51,51,51);font-family:'Arial Unicode MS',Arial,sans-serif;font-size:14px;line-height:20px">

<li>There should be a sufficient number of core reviewers who have volunteered to go <b>above and beyond</b> their typical reviewer workload (indicated by their +2) to review the relevant patches. A "sufficient number" is dependent on the individual spec and the scope of the change.</li>

<li>If reviewers have said they'll be reviewing a spec's patches <b>instead</b> of patches they'd review otherwise, that doesn't help much and is actually harmful to the overall project.</li></ul></div><div>

<br></div><div>So if the 10 specs you +2ed, don't have code posted until late, you should *not* be sitting idle, you should be doing regular code reviews for non-blueprints.</div><div><br></div><div><br></div><div>While I cannot speak for the dynamics of the tripleo team, if this were to be adopted in nova I would not +2 any blueprints as I don't think I can commit to *guaranteeing* I will have even more review bandwidth, I can make best efforts but a personally cannot guarantee. </div>

<div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">
<br>
In terms of my efficiency as a reviewer it is really in my interests<br>
to approve more work than I can reasonably expect to review, and then<br>
prioritize the review activity at time stuff appears for review. Some<br>
stuff that's approved will ultimately miss out, but I think that is<br>
perfectly acceptable. People can improve their chances of not missing<br>
out by getting their code up for review sooner than their competitors<br>
which is a useful carrot IMHO.<br>
<br>
<br>
Regards,<br>
Daniel<br>
<span class=""><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>
</font></span><div class=""><div class="h5"><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>