<div dir="ltr">Dmitry,<div><br></div><div>I've appreciated the feedback on my patches from your team and the work they are doing, it's great that everyone is working together better now. I think getting more puppet core reviewers is certainly on the horizon and will happen with continued effort, it just takes time and trust. But its "on the radar" to use a analogy.</div><div><br></div><div>As for your specific issue with the swift patch, I don't know enough about ring builder to decide whether the author or the swift reviewer is right so I was hoping he (the swift core) would follow-up to your comment. The puppet code itself is fine. I've also asked someone on my team who is a swift expert (but not a puppet core) to take a look and weigh-in. I don't know what our official policy is, but I would expect your author to reach out to the person who left the -1 and attempt to resolve it with them before one of us would essentially override the -1.<br><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Tue, Aug 18, 2015 at 12:33 AM, Dmitry Borodaenko <span dir="ltr"><<a href="mailto:dborodaenko@mirantis.com" target="_blank">dborodaenko@mirantis.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Two weeks ago, I flagged the patch sets to commits ratio as the biggest<br>
problem that Fuel contributors to Puppet OpenStack should address, and<br>
over the past two weeks the situation has improved dramatically. In<br>
first and second week of August, 19 of our commits were merged in<br>
upstream, bringing our patch sets per commit ratio from 19 down to 5.6<br>
(while average for Puppet OpenStack during that period was 6.5). With<br>
the share of patch sets pushed by Fuel developers remaining at roughly<br>
the same level (15.9% vs 17.4%), I think it's safe to call this problem<br>
solved. Simply awesome!<br>
<br>
Comparing last 30 days contribution stats vs same numbers two weeks ago:<br>
<br>
Bogdan Dobrelia (#3 reviewer!): 67.2% -> 66.3% (disagreements 4.9% -> 3.6%)<br>
Denis Egorenko: 97% -> 87.5% (disagreements 12.1% -> 13.9%)<br>
Vasyl Saienko: 100% -> 96.4% (disagreements 16.7% -> 10.7%)<br>
Ivan Berezovskiy: 100% -> 92.3% (disagreements 0% -> 3.8%)<br>
Sergey Kolekonov: 91.7% -> 95.7% (disagreements 8.3% -> 13%)<br>
Max Yatsenko: n/a -> 100% (disagreements n/a -> 17.4%)<br>
Alex Schultz: 80% -> 80% (disagreements 20% -> 26.7%)<br>
Bartlomiej Piotrowski: n/a -> 100% (disagreements n/a -> 12.5%)<br>
Sergii Golovatiuk: 100% -> 100% (disagreements 33.3% -> 0%)<br>
<br>
Bogdan continues to set the example and improve his numbers, which is<br>
not surprising considering that he's also the top reviewer in<br>
fuel-library. I think puppet-nova and puppet-neutron teams should<br>
seriously consider nominating him for core, he already tops reviewers<br>
lists for these modules.<br>
<br>
Denis, Vasyl, and Ivan are not there yet, but they all have noticeably<br>
increased both number and quality of their reviews, keep it up guys!<br>
<br>
Numbers for other top reviewers are uneven and small enough for noise to<br>
overtake meaningful data, all I can recommend here is to watch your<br>
disagreements and learn from them. A ratio above 10% can mean one of<br>
three things: 1) you're not doing enough reviews so even a handful of<br>
disagreements sticks out -- do more reviews and this will improve on its<br>
own; 2) you're missing problems with the code that other reviewers find<br>
unacceptable -- try to be more attentive and watch for the things that<br>
you've been missing; 3) you disagree with majority on how some things<br>
should be done -- discuss your differences on IRC or ML and figure out a<br>
consensus.<br>
<br>
Moving on to other numbers, weekly IRC meetings participation remains<br>
good:<br>
<br>
Aug-4: 4 of 15 participants, 22 of 162 lines<br>
Aug-11: 6 of 16 participants, 62 of 192 lines<br>
<br>
Unfortunately it's not all roses and unicorns, last week I flagged a<br>
stuck review that I think has been mishandled by Puppet OpenStack team<br>
[0][1], and it still remains in the same state.<br>
<br>
[0] <a href="https://review.openstack.org/198695" rel="noreferrer" target="_blank">https://review.openstack.org/198695</a><br>
[1] <a href="http://eavesdrop.openstack.org/meetings/puppet_openstack/2015/puppet_openstack.2015-08-11-15.00.log.html#l-140" rel="noreferrer" target="_blank">http://eavesdrop.openstack.org/meetings/puppet_openstack/2015/puppet_openstack.2015-08-11-15.00.log.html#l-140</a><br>
<br>
On July 8, patch set 2 has passed CI. It received 2 +1 votes from other<br>
Fuel developers on July 10 and 29, but remained ignored by upstream<br>
reviewers until August 10, more than a month after the current patch set<br>
was posted. Then, a Swift core reviewer left a -1 disagreeing with the<br>
intent of the patch, and even though patch author posted a rebuttal a<br>
day later, the patch remains stuck and untouched for yet another week.<br>
<br>
It's a one-off case that does not outweigh the positive trends I've<br>
outlined above, but even one stuck patch that fixes a critical bug is<br>
enough to justify a fork.<br>
<br>
Speaking of forks, we managed to un-fork 9 upstream modules [2] with<br>
puppet-librarian-simple before Fuel 7.0 soft code freeze has kicked in.<br>
<br>
[2] <a href="https://github.com/stackforge/fuel-library/blob/master/deployment/Puppetfile" rel="noreferrer" target="_blank">https://github.com/stackforge/fuel-library/blob/master/deployment/Puppetfile</a><br>
<br>
That's 9 times more than my conservative estimate of converting at least<br>
1 module to librarian in 7.0, but these were the easiest and least<br>
deviated from upstream. We've still got 50 more modules to convert in<br>
Fuel 8.0, many will require commits to be merged in upstream before they<br>
can be completely un-forked. Having such commits wait for a month at a<br>
time only to be summarily rejected puts this effort at risk, lets figure<br>
out what went wrong this time and come up with a way to prevent this<br>
from happening again.<br>
<br>
Thanks,<span class="HOEnZb"><font color="#888888"><br>
<br>
-- <br>
Dmitry Borodaenko<br>
<br>
__________________________________________________________________________<br>
OpenStack Development Mailing List (not for usage questions)<br>
Unsubscribe: <a href="http://OpenStack-dev-request@lists.openstack.org?subject:unsubscribe" rel="noreferrer" target="_blank">OpenStack-dev-request@lists.openstack.org?subject:unsubscribe</a><br>
<a href="http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev" rel="noreferrer" target="_blank">http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev</a><br>
</font></span></blockquote></div><br></div>