openstack team mailing list archive

[Mark McLoughlin <markmc@xxxxxxxxxx>]

On Tue, 2012-05-08 at 15:44 -0700, Vishvananda Ishaya wrote:

> The -2 issue is a good point. I personally treat a -1 (or +1) from the
> author of a given piece of code quite strongly when I do reviews, but
> you're right that the -1 could be more trivially overridden.

Coincidentally, dprince and I discussed this a bit yesterday.

I rarely -2, because I see it as a strong veto which blocks the patch or
later revisions of the patch until I remove the -2. Maybe it's just the
fact that I know I'm likely to be slow to come back and review later
revisions of a patch that I've put a -2 on. Maybe I should just fix
that :-)

On the flip-side, I respect (and expect others to respect) a -1 from a
core reviewer when reviewing later revisions of a patch - i.e. if I see
a -1 from a core reviewer, then I check that the later revisions of the
patch have actually addressed the previous concerns. If core reviewers'
concerns are consistently ignored, that'd definitely cause me to pull
out the big old -2.

> The removal is primarily to keep core a manageable size.  We currently
> have 25 core members and still have many patches that are not being
> quickly reviewed.  Giving too many people the ability to approve
> patches leads to inconsistency in code and the review process.  It
> seems like overkill to have > 20 people. I expect this number to
> decrease further if out plans to create substystem branches
> materialize.

I agree, FWIW.

20 seems like the right number, more than that and I think folks assume
"someone else will do it", core membership isn't for life, the
requirement that core members are actually active is a great incentive
to do reviews, folks can easily be re-added again later, etc. etc.

Cheers,
Mark.