On 9/2/15 5:19 AM, Gorka Eguileor wrote: > On Tue, Sep 01, 2015 at 09:30:26AM -0600, John Griffith wrote: >> On Tue, Sep 1, 2015 at 5:57 AM, Tom Barron <t...@dyncloud.net> wrote: >> >>> [Yesterday while discussing the following issue on IRC, jgriffith >>> suggested that I post to the dev list in preparation for a discussion in >>> Wednesday's cinder meeting.] >>> >>> Please take a look at the 10 "Low" priority reviews in the cinder >>> Liberty 3 etherpad that were punted to Mitaka yesterday. [1] >>> >>> Six of these *never* [2] received a vote from a core reviewer. With the >>> exception of the first in the list, which has 35 patch sets, none of the >>> others received a vote before Friday, August 28. Of these, none had >>> more than -1s on minor issues, and these have been remedied. >>> >>> Review https://review.openstack.org/#/c/213855 "Implement >>> manage/unmanage snapshot in Pure drivers" is a great example: >>> >>> * approved blueprint for a valuable feature >>> * pristine code >>> * passes CI and Jenkins (and by the deadline) >>> * never reviewed >>> >>> We have 11 core reviewers, all of whom were very busy doing reviews >>> during L3, but evidently this set of reviews didn't really have much >>> chance of making it. This looks like a classic case where the >>> individually rational priority decisions of each core reviewer >>> collectively resulted in starving the Low Priority review queue. >>> > > I can't speak for other cores, but in my case reviewing was mostly not > based on my own priorities, I reviewed patches based on the already set > priority of each patch as well as patches that I was already > reviewing. > > Some of those medium priority patches took me a lot of time to review, > since they were not trivial (some needed some serious rework). As for > patches I was already reviewing, as you can imagine it wouldn't be fair > to just ignore a patch that I've been reviewing for some time just when > it's almost ready and the deadline is closing in. >
That's why I said that this situation is an outcome of individually rational decisions. It should be clear that none of this is intended as a complaint about reviewers or reviewer's performance. > Having said that I have to agree that those patches didn't have much > chances, and I apologize for my part on that. While it is no excuse I > have to agree with jgriffith when he says that those patches should have > pushed cores for reviews (even if this is clearly not the "right" way to > manage it). No apology required! > >>> One way to remedy would be for the 11 core reviewers to devote a day or >>> two to cleaning up this backlog of 10 outstanding reviews rather than >>> punting all of them out to Mitaka. >>> >>> Thanks for your time and consideration. >>> >>> Respectfully, >>> >>> -- Tom Barron >>> >>> [1] https://etherpad.openstack.org/p/cinder-liberty-3-reviews >>> [2] At the risk of stating the obvious, in this count I ignore purely >>> procedural votes such as the final -2. >>> >>> __________________________________________________________________________ >>> 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 >>> >> >> ​Thanks Tom, this is sadly an ongoing problem every release. I think we >> have a number of things we can talk about at the summit to try and >> make some of this better. I honestly think that if people were to >> actually "use" launchpad instead of creating tracking etherpads >> everywhere it would help. What I mean is that there is a ranked >> targeting of items in Launchpad and we should use it, core team >> members should know that as the source of truth and things that must >> get reviewed. >> > > I agree, we should use Launchpad's functionality to track BPs and Bugs > targeted for each milestone, and maybe we can discuss on a workflow that > helps us reduce starvation at the same time that helps us keep track of > code reviewers responsible for each item. > > Just spitballing here, but we could add to BP's work items and Bug's > comments what core members will be responsible for reviewing related > patches. Although this means cores will have to check this on every > review they do that has a BP or Bug number, so if there are already 2 > cores responsible for that feature they should preferably just move on > to other patches and if there are not 2 core reviewers they should add > themselves to LP. This way patch owners know who they should bug for > reviews on their patches and if there are no core reviewers for them > they should look for some (or wait for them to "claim" that > bug/feature). > >> As far as Liberty and your patches; Yesterday was the freeze point, the >> entire Cinder team agreed on that (yourself included both at the mid-cycle >> meet up and at the team meeting two weeks ago when Thingee reiterated the >> deadlines). If you noticed last week that your patches weren't going >> anywhere YOU should've wrangled up some reviews. >> >> Furthermore, I've explained every release for the last 3 or 4 years that >> there's no silver bullet, no magic process when it come to review >> throughput. ESPECIALLY when it comes to the 3'rd milestone. You can try >> landing strips, priority listed etherpads, sponsors etc etc but the fact is >> that things happen, the gate slows down (or we completely break on the >> Cinder side like we did yesterday). This doesn't mean "oh, well then you >> get another day or two", it means stuff happens and it sucks but first >> course of action is drop low priority items. It just means if you really >> wanted it you probably should've made it happen earlier. Just so you know, >> I run into this every release as well. I had a number of things in >> progress that I had hoped to finish last week and yesterday, BUT my >> priority shifted to trying to help get the cinder patches back on track and >> get the items in Launchpad updated to actually reflect something that was >> somewhat possible. >> >> The only thing that works is "submit early and review often" it's simple. >> > > While this is true, sometimes it's not a matter of submitting early, > because some patches just keep getting ignored and when deadline closes > in low priority patches will always have higher priority patches that > will go before them, unless we set a workflow that will give them a > better chance of getting reviews. > >> Finally, I pointed out to you yesterday that we could certainly discuss as >> a team what to do with your patches. BUT that given how terribly far >> behind we were in the process that I wanted reviewers to focus on medium, >> high and critical prioritized items. That's what prioritization's are for, >> it means when crunch time hits and things hit the fan it's usually the >> "low" priority things that get bumped. >> >> Thanks, > > I am not against discussing this, but in all fairness we should discuss > *everybody's* patches that were targeted for L3, not just tbarron's. Completely agree, and I hope that everyone understands that this is the way I've been trying to frame the issue myself. -- Tom > > Cheers, > Gorka. > __________________________________________________________________________ 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