2015-09-02 17:19 GMT+08:00 Gorka Eguileor <gegui...@redhat.com>: > 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. > > 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). > >> > 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).
Like Gorka's idea here. This cloud be a better way to relieve this issue. I hope patch owners could require cores' help via IRC or something else if there are no core reviewers. > >> 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. > > 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 -- Best Wishes For You! __________________________________________________________________________ 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