Open Stack

On 12/07/2014 12:02 PM, Jay Pipes wrote:
> On 12/07/2014 04:19 AM, Michael Still wrote:
>> On Sun, Dec 7, 2014 at 7:03 PM, Gary Kotton <gkotton at vmware.com> wrote:
>>> On 12/6/14, 7:42 PM, "Jay Pipes" <jaypipes at gmail.com> wrote:
>>
>> [snip]
>>
>>>> -1 on pixelbeat, since he's been active in reviews on
>>>> various things AFAICT in the last 60-90 days and seems to be still a
>>>> considerate reviewer in various areas.
>>>
>>> I agree -1 for Padraig
>>
>> I'm going to be honest and say I'm confused here.
>>
>> 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.

Maybe. I'm kind of torn on it.

I think we need to separate "providing insightful reviews" with
"actively engaged in Nova". I feel like there are tons of community
members that provide insightful reviews that we hold a patch until we've
seen their relevant +1 in an area of their expertise. If our concern is
missing expertise, then I don't think this changes things.

I could go either way on this one in particular. But I'm also happy to
drop and move forward. Padraig's commit history in OpenStack atm show's
that his focus right now isn't upstream. He's not currently very active
in IRC regularly, on the ML, triaging bugs, or fixing bugs, which are
all ways we know folks are engaged enough to have a feel where the norms
of Nova have evolved. Which is cool, folks change focus.

I think in the past we've erred very heavily on making it tough to let
people into the core reviewer team because it's so hard to remove
people. Which doesn't help us grow, we stagnate. I think the fear of a
fight on removal of core reviewers every time makes people even more
cautious in supporting adds.

Maybe this is erring in the other direction, but I'm happy to take
Michael's judgement call on that that it isn't. If Padraig gets more
engaged, I'd be happy adding him back in.

	-Sean

-- 
Sean Dague
http://dague.net