Open Stack

 ---- On Thu, 03 Jan 2019 22:51:55 +0900 Sean McGinnis <sean.mcginnis at 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 
 >  
 >