Open Stack

Dmitry,

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.

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.


On Tue, Aug 18, 2015 at 12:33 AM, Dmitry Borodaenko <
dborodaenko at mirantis.com> wrote:

> Two weeks ago, I flagged the patch sets to commits ratio as the biggest
> problem that Fuel contributors to Puppet OpenStack should address, and
> over the past two weeks the situation has improved dramatically. In
> first and second week of August, 19 of our commits were merged in
> upstream, bringing our patch sets per commit ratio from 19 down to 5.6
> (while average for Puppet OpenStack during that period was 6.5). With
> the share of patch sets pushed by Fuel developers remaining at roughly
> the same level (15.9% vs 17.4%), I think it's safe to call this problem
> solved. Simply awesome!
>
> Comparing last 30 days contribution stats vs same numbers two weeks ago:
>
> Bogdan Dobrelia (#3 reviewer!): 67.2% -> 66.3% (disagreements 4.9% -> 3.6%)
> Denis Egorenko: 97% -> 87.5% (disagreements 12.1% -> 13.9%)
> Vasyl Saienko: 100% -> 96.4% (disagreements 16.7% -> 10.7%)
> Ivan Berezovskiy: 100% -> 92.3% (disagreements 0% -> 3.8%)
> Sergey Kolekonov: 91.7% -> 95.7% (disagreements 8.3% -> 13%)
> Max Yatsenko: n/a -> 100% (disagreements n/a -> 17.4%)
> Alex Schultz: 80% -> 80% (disagreements 20% -> 26.7%)
> Bartlomiej Piotrowski: n/a -> 100% (disagreements n/a -> 12.5%)
> Sergii Golovatiuk: 100% -> 100% (disagreements 33.3% -> 0%)
>
> Bogdan continues to set the example and improve his numbers, which is
> not surprising considering that he's also the top reviewer in
> fuel-library. I think puppet-nova and puppet-neutron teams should
> seriously consider nominating him for core, he already tops reviewers
> lists for these modules.
>
> Denis, Vasyl, and Ivan are not there yet, but they all have noticeably
> increased both number and quality of their reviews, keep it up guys!
>
> Numbers for other top reviewers are uneven and small enough for noise to
> overtake meaningful data, all I can recommend here is to watch your
> disagreements and learn from them. A ratio above 10% can mean one of
> three things: 1) you're not doing enough reviews so even a handful of
> disagreements sticks out -- do more reviews and this will improve on its
> own; 2) you're missing problems with the code that other reviewers find
> unacceptable -- try to be more attentive and watch for the things that
> you've been missing; 3) you disagree with majority on how some things
> should be done -- discuss your differences on IRC or ML and figure out a
> consensus.
>
> Moving on to other numbers, weekly IRC meetings participation remains
> good:
>
> Aug-4: 4 of 15 participants, 22 of 162 lines
> Aug-11: 6 of 16 participants, 62 of 192 lines
>
> Unfortunately it's not all roses and unicorns, last week I flagged a
> stuck review that I think has been mishandled by Puppet OpenStack team
> [0][1], and it still remains in the same state.
>
> [0] https://review.openstack.org/198695
> [1]
> http://eavesdrop.openstack.org/meetings/puppet_openstack/2015/puppet_openstack.2015-08-11-15.00.log.html#l-140
>
> On July 8, patch set 2 has passed CI. It received 2 +1 votes from other
> Fuel developers on July 10 and 29, but remained ignored by upstream
> reviewers until August 10, more than a month after the current patch set
> was posted. Then, a Swift core reviewer left a -1 disagreeing with the
> intent of the patch, and even though patch author posted a rebuttal a
> day later, the patch remains stuck and untouched for yet another week.
>
> It's a one-off case that does not outweigh the positive trends I've
> outlined above, but even one stuck patch that fixes a critical bug is
> enough to justify a fork.
>
> Speaking of forks, we managed to un-fork 9 upstream modules [2] with
> puppet-librarian-simple before Fuel 7.0 soft code freeze has kicked in.
>
> [2]
> https://github.com/stackforge/fuel-library/blob/master/deployment/Puppetfile
>
> That's 9 times more than my conservative estimate of converting at least
> 1 module to librarian in 7.0, but these were the easiest and least
> deviated from upstream. We've still got 50 more modules to convert in
> Fuel 8.0, many will require commits to be merged in upstream before they
> can be completely un-forked. Having such commits wait for a month at a
> time only to be summarily rejected puts this effort at risk, lets figure
> out what went wrong this time and come up with a way to prevent this
> from happening again.
>
> Thanks,
>
> --
> Dmitry Borodaenko
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: OpenStack-dev-request at lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstack.org/pipermail/openstack-dev/attachments/20150819/6d6cc192/attachment.html>