---- On Thu, 03 Jan 2019 22:51:55 +0900 Sean McGinnis <sean.mcginnis@gmx.com> wrote ----
On Fri, Dec 28, 2018 at 11:04:41AM +0530, Surya Singh wrote:
Dear All,
There are many occasion when we want to priorities some of the patches whether it is related to unblock the gates or blocking the non freeze patches during RC.
So adding the Review-Priority will allow more precise dashboard. As Designate and Cinder projects already experiencing this[1][2] and after discussion with Jeremy brought this to ML to interact with these team before landing [3], as there is possibility that reapply the priority vote following any substantive updates to change could make it more cumbersome than it is worth.
With Cinder this is fairly new, but I think it is working well so far. The oddity we've run into, that I think you're referring to here, is how those votes carry forward with updates.
I set up Cinder with -1, +1, and +2 as possible priority votes. It appears when
This idea looks great and helpful especially for blockers and cycle priority patches to get regular review bandwidth from Core or Active members of that project. IMO only +ve votes are more appropriate for this label. -1 is little confusing for many reasons like what is the difference between Review-Priority -1 and Code-Review -2 ? Review-Priority -1 means, it is less priority than 0/not labelled (explicitly setting any patch very less priority). After seeing Cinder dashboard, I got to know that -1 is used to block the changes due to procedural or technical reason. But that can be done by -2 on Code-Review label. Keeping Review-Priority label only for priority set makes it more clear which is nothing but allowing only +ve votes for this label. Personally, I prefer only a single vote set which can be +1 to convey that these are the set of changes priority for review but having multiple +ve vote set as per project need/interest is all fine. -gmann
a patchset is updates, the -1 and +2 carry forward. But for some reason we can't get the +1 to be sticky.
So far, that's just a slight inconvenience. It would be great if we can figure out a way to have them all be sticky, but if we need to live with reapplying +1 votes, that's manageable to me.
The one thing I have been slightly concerned with is the process around using these priority votes. It hasn't been an issue, but I could see a scenario where one core (in Cinder we have it set up so all cores can use the priority voting) has set something like a procedural -1, then been pulled away or is absent for an extended period. Like a Workflow -2, another core cannot override that vote. So until that person is back to remove the -1, that patch would not be able to be merged.
Granted, we've lived with this with Workflow -2's for years and it's never been a major issue, but I think as far as centralizing control, it may make sense to have a separate smaller group (just the PTL, or PTL and a few "deputies") that are able to set priorities on patches just to make sure the folks setting it are the ones that are actively tracking what the priorities are for the project.
Anyway, my 2 cents. I can imagine this would work really well for some teams, less well for others. So if you think it can help you manage your project priorities, I would recommend giving it a shot and seeing how it goes. You can always drop it if it ends up not being effective or causing issues.
Sean