[openstack-dev] [heat] Core criteria, review stats vs reality

Clint Byrum clint at fewbar.com
Mon Dec 9 19:01:20 UTC 2013


Excerpts from Steven Hardy's message of 2013-12-09 03:31:36 -0800:
> Hi all,
> 
> So I've been getting concerned about $subject recently, and based on some
> recent discussions so have some other heat-core folks, so I wanted to start
> a discussion where we can agree and communicate our expectations related to
> nomination for heat-core membership (becuase we do need more core
> reviewers):
> 
> The issues I have are:
> - Russell's stats (while very useful) are being used by some projects as
>   the principal metric related to -core membership (ref TripleO's monthly
>   cull/name&shame, which I am opposed to btw).  This is in some cases
>   encouraging some stats-seeking in our review process, IMO.
> 

This is quite misleading, so please do put the TripleO reference in
context:

http://lists.openstack.org/pipermail/openstack-dev/2013-October/016186.html
http://lists.openstack.org/pipermail/openstack-dev/2013-October/016232.html

Reading the text of those two I think you can see that while the stats
are a tool Robert is using to find the good reviewers, it is not "the
principal metric".

I also find it quite frustrating that you are laying accusations of
"stats-seeking" without proof. That is just spreading FUD. I'm sure that
is not what you want to do, so I'd like to suggest that we not accuse our
community of any kind of "cheating" or "gaming" of the system without
actual proof. I would also suggest that these accusations be made in
private and dealt with directly rather than as broad passive-aggressive
notes on the mailing list.

> - Review quality can't be measured mechanically - we have some folks who
>   contribute fewer, but very high quality reviews, and are also very active
>   contributors (so knowledge of the codebase is not stale).  I'd like to
>   see these people do more reviews, but removing people from core just
>   because they drop below some arbitrary threshold makes no sense to me.
>

Not sure I agree that it absolutely can't, but it certainly isn't
something these stats are even meant to do.

We other reviewers must keep tabs on our aspiring core reviewers and
try to rate them ourselves based on whether or not they're spotting the
problems we would spot, and whether or not they're also upholding the
culture we want to foster in our community. We express our rating of
these people when voting on a nomination in the mailing list.

So what you're saying is, there is more to our votes than the mechanical
number. I'd agree 100%. However, I think the numbers _do_ let people
know where they stand in one very limited aspect versus the rest of the
community.

I would actually find it interesting if we had a meta-gerrit that asked
us to "review the reviews". This type of system works fantastically for
stackexchange. That would give us a decent mechanical number as well.

> So if you're aiming for heat-core nomination, here's my personal wish-list,
> but hopefully others can proide their input and we can update the wiki with
> the resulting requirements/guidelines:
> 
> - Make your reviews high-quality.  Focus on spotting logical errors,
>   reducing duplication, consistency with existing interfaces, opportunities
>   for reuse/simplification etc.  If every review you do is +1, or -1 for a
>   trivial/cosmetic issue, you are not making a strong case for -core IMHO.
> 

Disagree. I am totally fine having somebody in core who is really good
at finding all of the trivial cosmetic issues. Those should mean that
the second +2'er of their code is looking at code free of trivial and
cosmetic issues that distract from the bigger issues.

> - Send patches.  Some folks argue that -core membership is only about
>   reviews, I disagree - There are many aspects of reviews which require
>   deep knowledge of the code, e.g spotting structural issues, logical
>   errors caused by interaction with code not modified by the patch,
>   effective use of test infrastructure, etc etc.  This deep knowledge comes
>   from writing code, not only reviewing it.  This also gives us a way to
>   verify your understanding and alignment with our sylistic conventions.
> 

The higher the bar goes, the less reviewers we will have. There are
plenty of people that will find _tons_ of real issues but won't submit
very many patches if any. However, I think there isn't any value in
arguing over this point as most of our reviewers are also submitting
patches already.

> - Fix bugs.  Related to the above, help us fix real problems by testing,
>   reporting bugs, and fixing them, or take an existing bug and post a patch
>   fixing it.  Ask an existing team member to direct you if you're not sure
>   which bug to tackle.  Sending patches doing trivial cosmetic cleanups is
>   sometimes worthwhile, but make sure that's not all you do, as we need
>   -core folk who can find, report, fix and review real user-impacting
>   problems (not just new features).  This is also a great way to build
>   trust and knowledge if you're aiming to contribute features to Heat.
>

There's a theme running through this message which is suggesting "we do
not value small contributions as much as big ones.". Core needs small
contributions too. I personally trust people to step back from a patch
and say "I am not versed enough in this code to +2 this." Meanwhile those
same people can help push patches through when they are in fact trivial,
and thus lighten the load for folk with a heavier burden of reviewing
more complicated patches.

> - Engage in discussions related to the project (here on the ML, helping
>   users on the general list, in #heat on Freenode, attend our weekly
>   meeting if it's not an anti-social time in your TZ)
> 

Yes please!

> Anyone have any more thoughts to add here?
> 

Please let's be inclusive. OpenStack would be nothing if it just allowed
the best and brightest to contribute. The new and unpolished will become
the best and brightest not by keeping their training wheels on or swimming
in the kiddie pool, but by working hard to keep up with everyone else.



More information about the OpenStack-dev mailing list