<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Apr 7, 2015 at 11:12 AM, Tim Bell <span dir="ltr"><<a href="mailto:Tim.Bell@cern.ch" target="_blank">Tim.Bell@cern.ch</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=""><div class="h5">> -----Original Message-----<br>
> From: James Bottomley [mailto:<a href="mailto:James.Bottomley@HansenPartnership.com">James.Bottomley@HansenPartnership.com</a>]<br>
> Sent: 07 April 2015 19:03<br>
> To: Michael Still<br>
> Cc: OpenStack Development Mailing List (not for usage questions)<br>
> Subject: [openstack-dev] Fixing the Nova Core Reviewer Frustration [was Re:<br>
> [Nova] PTL Candidacy]<br>
><br>
> On Tue, 2015-04-07 at 11:27 +1000, Michael Still wrote:<br>
> > Additionally, we have consistently asked for non-cores to help cover<br>
> > the review load. It doesn't have to be a core that notices a problem<br>
> > with a patch -- anyone can do that. There are many people who do help<br>
> > out with non-core reviews, and I am thankful for all of them. However,<br>
> > I keep meeting people who complain about review delays, but who don't<br>
> > have a history of reviewing themselves. That's confusing and<br>
> > frustrating to me.<br>
><br>
> I can understand why you're frustrated, but not why you're surprised:<br>
> the process needs to be different.  Right now the statement is that for a patch<br>
> series to be accepted it has to have a positive review from a core plus one other,<br>
> however the "one other" can be a colleague, so it's easy.  The problem, as far as<br>
> submitters see it, is getting that Core Reviewer.  That's why so much frenzy<br>
> (which contributes to your<br>
> frustration) goes into it.  And why all the complaining which annoys you.<br>
><br>
> To fix the frustration, you need to fix the process:  Make the cores more of a<br>
> second level approver rather than a front line reviewer and I predict the frenzy<br>
> to get a core will go down and so will core frustration.  Why not require a +1<br>
> from one (or even more than one) independent (for some useful value of<br>
> independent) reviewer before the cores will even look at it?  That way the cores<br>
> know someone already thought the patch was good, so they're no longer being<br>
> pestered to review any old thing and the first job of a submitter becomes to find<br>
> an independent reviewer rather than go bother a core.<br>
><br>
<br>
</div></div>If I take a case that we were very interest in (<a href="https://review.openstack.org/#/c/129420/" target="_blank">https://review.openstack.org/#/c/129420/</a>) for nested project quota, we seemed to need two +2s from core reviewers on the spec.<br>
<br>
There were many +1s but these did not seem to result in an increase in attention to get the +2s. Initial submission of the spec was in October but we did not get approval till the end of January.<br></blockquote><div><br></div><div>I'll bite. As one of the '+2's on your spec, I don't think this is a fair characterization of what happened.</div><div><br></div><div>* This spec took 36 revisions, but most of those were back to back in the same day with no feedback. So this actually translates to about 9 or so reviewed revisions. For a big spec like this I don't think that is too bad</div><div>* The first set of feedback came a few days after the first draft was posted, which isn't too bad.</div><div>* The first +1 was on December 11th. So that means from the first +1 to being merged was  December to January, not October to January.</div><div>* The 'many +1s' were all from the group proposing the patch, and as reviewer with +2 power, I generally discount those +1s since they are inherently fairly biased. Although a lack of +1s from the teammates of the author is an even worse sign. So I see a lack of +1s from the group as a bad sign.</div><div>* The final revision was posted on December 18th (ignoring the trivial cleanup/rebase I ran) and this finally landed on January 26th. This is where I think we could have done better, it shouldn't have taken over a month to get 2 +2s  at this point.</div><div>* The fact that the RST didn't render properly until I went through and fixed it myself was a bad sign:  <a href="https://review.openstack.org/#/c/129420/35..36/specs/kilo/approved/nested-quota-driver-api.rst,cm">https://review.openstack.org/#/c/129420/35..36/specs/kilo/approved/nested-quota-driver-api.rst,cm</a> </div><div><br></div><div>In short, I think we dropped the ball a little bit on getting the final +2 on this, but by the time this was really ready it was already very late in the cycle.</div><div><br></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>
Unfortunately, we were unable to get the code into the right shape after the spec approval to make it into Kilo.<br>
<br>
One of the issues for the academic/research sector is that there is a significant resource available from project resources but these are time limited. Thus, if a blueprint and code commit cannot be completed within the window for the project, the project ends and resources to complete are no longer available. Naturally, rejections on quality grounds such as code issues or lack of test cases is completely reasonable but the latency time can extend the time to delivery significantly.<br></blockquote><div><br></div><div>This is the first time I heard of any time window issues or this blueprint, so I am not sure how you expected us to work within said window. In cases where there are some time constraints like this, you should just raise awareness of this early on in the process so we can work around it.</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>
Luckily, in this case, the people concerned are happy to continue to completion (and the foundation is sponsoring the travel for the summit too) but this would not always be the case.<br>
<br>
Tim<br>
<br>
> James<br>
><br>
><br>
><br>
> _________________________________________________________________<br>
<div class=""><div class="h5">> _________<br>
> OpenStack Development Mailing List (not for usage questions)<br>
> Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</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>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</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>