openstack team mailing list archive

[Matt Dietz <matt.dietz@xxxxxxxxxxxxx>]

The problem here is there are two opposing points: the idea that there are
too many core reviewers, and the idea that patches aren't being reviewed
"fast enough." 

*Drags a yak into the room* Beyond that, what makes 20 better than 25, or
15, especially in light of the fact that we're not happy with how quickly
reviews are moving?

It seems like the answer here isn't just quantity, but a matter of focus
and relative skill sets. I'm a core developer but, beyond code quality, I
couldn't review in the best interests of functionality in and around KVM.
Conversely, I'm sure most core developers couldn't do any better for
XenServer. I liked the idea of lieutenants when it first came up almost 2
years ago. Have we talked about reinstating that?


On 5/9/12 5:34 AM, "Mark McLoughlin" <markmc@xxxxxxxxxx> wrote:

>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.
>
>
>_______________________________________________
>Mailing list: https://launchpad.net/~openstack
>Post to     : openstack@xxxxxxxxxxxxxxxxxxx
>Unsubscribe : https://launchpad.net/~openstack
>More help   : https://help.launchpad.net/ListHelp