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

Steven Hardy shardy at redhat.com
Mon Dec 9 22:57:50 UTC 2013


On Tue, Dec 10, 2013 at 11:25:49AM +1300, Robert Collins wrote:
> On 10 December 2013 11:04, Steven Hardy <shardy at redhat.com> wrote:
> 
> >> So it's a gross mischaracterisation to imply that a democratic process
> >> aided by some [crude] stats has been reduced to name & shame, and a
> >> rather offensive one.
> >
> > Yes I have read your monthly core reviewer update emails[1] and I humbly
> > apologize if you feel my characterization of your process is offensive, it
> > certainly wasn't intended to be.
> 
> Thank; enough said here - lets move on :)
> 
> 
> >> I think you have a very different definition of -core to the rest of
> >> OpenStack. I was actually somewhat concerned about the '+2 Guard the
> >> gate stuff' at the summit because it's so easily misinterpreted - and
> >> there is a meme going around (I don't know if it's true or not) that
> >> some people are assessed - performance review stuff within vendor
> >> organisations - on becoming core reviewers.
> >>
> >> Core reviewer is not intended to be a gateway to getting features or
> >> ideas into OpenStack projects. It is solely a volunteered contribution
> >> to the project: helping the project accept patches with confidence
> >> about their long term integrity: providing explanation and guidance to
> >> people that want to contribute patches so that their patch can be
> >> accepted.
> >
> > We need core reviewers who:
> > 1. Have deep knowledge of the codebase (to identify non-cosmetic structural
> > issues)
> 
> mmm, core review is a place to identify really significant structural
> issues, but it's not ideal. Because - you do a lot of work before you
> push code for review, particularly if it's ones first contribution to
> a codebase, and that means a lot of waste when the approach is wrong.
> Agree that having -core that can spot this is good, but not convinced
> that it's a must.

So you're saying you would give approve rights to someone without
sufficient knowledge to recognise the implications of a patch in the
context of the whole tree?  Of course it's a must.

> > 2. Have used and debugged the codebase (to identify usability, interface
> > correctness or other stuff which isn't obvious unless you're using the
> > code)
> 
> If I may: 2a) Have deployed and used the codebase in production, at
> scale. This may conflict with 1) in terms of individual expertise.

Having folks involved with experience of running stuff in production is
invaluable I agree, I just meant people should have some practical
experience outside of the specific feature they may be working on.

> > 3. Have demonstrated a commitment to the project (so we know they
> > understand the mid-term goals and won't approve stuff which is misaligned
> > with those goals)
> 
> I don't understand this. Are you saying you'd turn down contributions
> that are aligned with the long term Heat vision because they don't
> advance some short term goal? Or are you saying you'd turn down
> contributions because they actively harm short term goals?

I'm saying people in a position to approve patches should be able to
decouple the short term requirements of e.g their employer, or the feature
they are interested in, from the long-term goals (e.g maintainability) of
the project.

We can, and have, turned down contributions because they offered short-term
solutions to problems which didn't make sense long term from an upstream
perspective (normally with discussion of suitable alternative approaches).

> Seems to me that that commitment to the project is really orthogonal
> to either of those things - folk may have different interpretations
> about what the project needs while still being entirely committed to
> it! Perhaps you mean 'shared understanding of current goals and
> constraints' ? Or something like that? I am niggling on this point
> because I wouldn't want someone who is committed to TripleO but
> focused on the big picture to be accused of not being committed to
> TripleO.

Yeah, shared understanding, I'm saying all -core reviewers should have, and
have demonstrated, some grasp of the big picture for the project they
control the gate for.

Steve



More information about the OpenStack-dev mailing list