<div dir="ltr"><br><div class="gmail_extra"><br><br><div class="gmail_quote">On Fri, Oct 11, 2013 at 6:02 AM, Alessandro Pilotti <span dir="ltr"><<a href="mailto:apilotti@cloudbasesolutions.com" target="_blank">apilotti@cloudbasesolutions.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"><br>
On Oct 11, 2013, at 14:15 , Sean Dague <<a href="mailto:sean@dague.net">sean@dague.net</a>><br>
<div class="im"> wrote:<br>
<br>
> On 10/10/2013 08:43 PM, Tim Smith wrote:<br>
> <snip><br>
>> Again, I don't have any vested interest in this discussion, except that<br>
>> I believe the concept of "reviewer karma" to be counter to both software<br>
>> quality and openness. In this particular case it would seem that the<br>
>> simplest solution to this problem would be to give one of the hyper-v<br>
>> team members core reviewer status, but perhaps there are consequences to<br>
>> that that elude me.<br>
><br>
> There are very deep consequences to that. The core team model, where you have 15 - 20 reviewers, but it only takes 2 to land code, only works when the core teams share a culture. This means they know, or are willing to learn, code outside their comfort zone. Will they catch all the bugs in that? nope. But code blindness hits everyone, and there are real implications for the overall quality and maintainability of a project as complicated as Nova if everyone only stays in their comfortable corner.<br>
><br>
> Also, from my experience in Nova, code contributions written by people that aren't regularly reviewing outside of their corner of the world are demonstrably lower quality than those who are. Reviewing code outside your specific area is also educational, gets you familiar with norms and idioms beyond what simple style checking handles, and makes you a better developer.<br>
<br>
<br>
</div>There's IMO a practical contradiction here: most people contribute code and do reviews on partitioned areas of OpenStack only. For example, Nova devs rarely commit on Neutron, so you can say that for a Nova dev the "confort zone" is Nova, but by your description, a fair amount of time should be spent in reviewing and learning all the OpenStack projects code, unless you want to limit the scope of this discussion to Nova, which does not make much sense when you work on a whole technology layer like in our case.<br>
<br>
On the contrary, as an example, our job as driver/plugin/agent mantainers brings us in contact will all the major projects codebases, with the result that we are learning a lot from each of them. Beside that, obviously a driver/plugin/agent dev spends normally time learning how similar solutions are implemented for other technologies already in the tree, which leads to further improvement in the code due to the same knowledge sharing that you are referring to.<br>
<div class="im"><br>
><br>
> We need to all be caring about the whole. That culture is what makes OpenStack long term sustainable, and there is a reason that it is behavior that's rewarded with more folks looking at your proposed patches. When people only care about their corner world, and don't put in hours on keeping things whole, they balkanize and fragment.<br>
><br>
> Review bandwidth, and people working on core issues, are our most constrained resources. If teams feel they don't need to contribute there, because it doesn't directly affect their code, we end up with this - <a href="http://en.wikipedia.org/wiki/Tragedy_of_the_commons" target="_blank">http://en.wikipedia.org/wiki/Tragedy_of_the_commons</a><br>
><br>
<br>
</div>This reminds me about how peer to peer sharing technologies work. Why don't we put some ratios, for example for each commit that a dev does at least 2-3 reviews of other people's code are required? Enforcing it wouldn't be that complicated. The negative part is that it might lead to low quality or fake reviews, but at least it could be easy to outline in the stats.<br>
<br>
One thing is sure: review bandwidth is the obvious bottleneck in today's OpenStack status. If we don't find a reasonably quick solution, the more OpenStack grows, the more complicated it will become, leading to even worse response times in merging bug fixes and limiting the new features that each new version can bring, which is IMO the negation of what a vital and dynamic project should be.<br>
</blockquote><div><br></div><div>Yes, review bandwidth is a bottleneck, and although there are some organizational changes that may help at the risk of changing our entire review process and culture (which perhaps we should consider?). The easiest solution is for everyone to do more reviews. For just one review a day you can make the whole project much stronger. Complaining about the review bandwidth issue while only doing 33 reviews in all of OpenStack [1] in the past 90 months (I don't mean to pick on you out here, you are just an example) doesn't seem right.</div>
<div><br></div><div>[1] <a href="http://www.russellbryant.net/openstack-stats/all-reviewers-90.txt">http://www.russellbryant.net/openstack-stats/all-reviewers-90.txt</a></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>
>From what I see on the Linux kernel project, which can be considered as a good source of inspiration when it comes to review bandwidth optimization in a large project, they have a pyramidal structure in the way in which the git repo origins are interconnected. This looks pretty similar to what we are proposing: teams work on specific areas with a topic mantainer and somebody merges their work at a higher level, with Linus ultimately managing the root repo.<br>
<br>
OpenStack is organized differently: there are lots of separate projects (Nova, Neutrom, Glance, etc) instead of a single one (which is a good thing), but I believe that a similar approach can be applied. Specific contributors can be nominated "core rewievers" on specific directories in the tree only and that would scale immediately the core review bandwidth.<br>
<br>
As a practical example for Nova: in our case that would simply include the following subtrees: "nova/virt/hyperv" and "nova/tests/virt/hyperv". Other projects didn't hit the review bandwidth limits yet as heavily as Nova did, but the same concept could be applied everywhere.<br>
<span class=""><font color="#888888"><br>
Alessandro<br>
</font></span><div class=""><div class="h5"><br>
<br>
<br>
<br>
> So it's really crazy to call OpenStack less open by having a culture that encourages people to actually work and help on the common parts. It's good for the project, as it keeps us whole; it's good for everyone working on the project, because they learn about more parts of OpenStack, and how their part fits in with the overall system; and it makes everyone better developers from learning from each other, on both sides of the review line.<br>
><br>
> -Sean<br>
><br>
> --<br>
> Sean Dague<br>
> <a href="http://dague.net" target="_blank">http://dague.net</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>
<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>