Open Stack

On Mon, Dec 8, 2014 at 4:02 AM, Jay Pipes <jaypipes at gmail.com> wrote:
> On 12/07/2014 04:19 AM, Michael Still wrote:

[snip]

>> We've always said we expect cores to maintain an average of two
>> reviews per day. That's not new, nor a rule created by me. Padraig is
>> a great guy, but has been working on other things -- he's done 60
>> reviews in the last 60 days -- which is about half of what we expect
>> from a core.
>>
>> Are we talking about removing the two reviews a day requirement? If
>> so, how do we balance that with the widespread complaints that core
>> isn't keeping up with its workload? We could add more people to core,
>> but there is also a maximum practical size to the group if we're going
>> to keep everyone on the same page, especially when the less active
>> cores don't generally turn up to our IRC meetings and are therefore
>> more "expensive" to keep up to date.
>>
>> How can we say we are doing our best to keep up with the incoming
>> review workload if all reviewers aren't doing at least the minimum
>> level of reviews?
>
> Personally, I care more about the quality of reviews than the quantity. That
> said, I understand that we have a small number of core reviewers relative to
> the number of open reviews in Nova (~650-700 open reviews most days) and
> agree with Dan Smith that 2 reviews per day doesn't sound like too much of a
> hurdle for core reviewers.
>
> The reason I think it's important to keep Padraig as a core is that he has
> done considerate, thoughtful code reviews, albeit in a smaller quantity. By
> saying we only look at the number of reviews in our estimation of keeping
> contributors on the core team, we are incentivizing the wrong behaviour,
> IMO. We should be pushing that the thought that goes into reviews is more
> important than the sheer number of reviews.
>
> Is it critical that we get more eyeballs reviewing code? Yes, absolutely it
> is. Is it critical that we get more reviews from core reviewers as well as
> non-core reviewers. Yes, absolutely.
>
> Bottom line, we need to balance between quality and quantity, and kicking
> out a core reviewer who has quality code reviews because they don't have
> that many of them sends the wrong message, IMO.

I agree that we need to maintain the quality of reviews. What I am
instead expecting is for core reviewers to spend enough of their work
day to get two high quality reviews done. That's a very low bar.

I am trying to balance the following constraints:

 - we aren't keeping up with reviews
 - a small number of cores are doing the majority of the work
 - there have been threats of forking drivers out of the nova code
base if we don't solve this and I really don't want to see that happen

There are other things happening behind the scenes as well -- we have
a veto process for current cores when we propose a new core. It has
been made clear to me that several current core members believe we
have reached "the maximum effective size" for core, and that they will
therefore veto new additions. Therefore, we need to make room in core
for people who are able to keep up with our review workload.

You know what makes me really sad? No one has suggested that perhaps
Padraig could just pick up his review rate a little. I've repeatedly
said we can re-add reviewers if that happens.

Michael

-- 
Rackspace Australia