On Fri, Feb 17, 2017 at 5:48 PM, Ian Cordasco <sigmaviru...@gmail.com> wrote: > -----Original Message----- > From: Thomas Herve <the...@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 __________________________________________________________________________ OpenStack Development Mailing List (not for usage questions) Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
