Open Stack

On Fri, Feb 17, 2017 at 5:48 PM, Ian Cordasco <sigmavirus24 at gmail.com> wrote:
> -----Original Message-----
> From: Thomas Herve <therve at redhat.com>
[snip]
>> Respecting the guidelines is totally fair, but review stats won't tell
>> you much, at least in my case: I barely do any stable reviews because
>> I don't have approve rights. In the case of Heat, 90% of the backports
>> are without conflicts, so stable reviews are just about verifying the
>> guidelines and that the patch matches what's in master.
>>
>> But, I've been working on Heat for 4 years, I made about 1400 reviews
>> on it, and I've been PTL. And the same for the other people that Zane
>> mentioned. I feel we should be trusted on stable branches.
>
> That seems like a very poor excuse - "I can't approve so I don't
> review". I'm a stable maintenance core because I was reviewing stable
> branch changes first. I had a good track record, and both the existing
> Glance stable maint core reviewers and the global team agreed I had
> displayed sound judgment for those.

It's not an excuse, I'm explaining why I don't do many stable reviews.
My time is valuable, and I don't do all the reviews that I could on
master already. I'd rather spend review time where I can move the
needle, instead on patches where ultimately that won't matter. If I
see a stable patch which doesn't make sense, I'll comment, but it's
very rare. On others, if that looks fine I don't do anything. Because
most contributors (on Heat at least) already made the effort to think
whether their change was backport worthy. And I don't chase stats.

> Without being able to assess the quality of your reviews, how should
> anyone else trust you with the stability of those branches?

You can assess the quality of my reviews on master. I don't see how
stable is so different. We can't break APIs, we can't change the DB
randomly, we can't break compatibility. The pain points (dependencies,
config options, features) are most of the time easy to spot.

(Also, Matt mentioned review stats. I could have 200 stable +1, that
would maybe look nice on paper, but not prove anything, if there is
anything to prove).

>> > There are reviewstats tools for seeing the stable review numbers for Heat, I
>> > haven't run that though to check against those proposed above, but it's
>> > probably something I'd do first before just adding a bunch of people.
>>
>> I appreciate your guidance and input, but shouldn't we decide our
>> stable maintainers, the same way we decide cores? The current list
>> contains at least one person that doesn't contribute anymore, so it's
>> not like it's super curated.
>
> This is how every other service team works (Nova, Keystone, Glance,
> etc.). Just because the global stable maint team hasn't removed an
> inactive person doesn't invalidate their assessment of potential core
> reviewers.

Saying that how other work is not a fantastic argument. We'd need to
know if that actually works for them.

At any rate, it's a matter of trust, a subject that comes from time to
time, and it's fairly divisive. In this case though, I find it ironic
that I can approve whatever garbage I want on master, it can make its
way into a release, but if I want a bugfix backported into another
branch, someone else has to supervise me.

-- 
Thomas