Open Stack

On 1/10/19 12:17 AM, Ghanshyam Mann wrote:
>   ---- 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.

I don't know if this was the reasoning behind Cinder's system, but I 
know some people object to procedural -2 because it's a big hammer to 
essentially say "not right now". It overloads the meaning of the vote in 
a potentially confusing way that requires explanation every time it's 
used. At least I hope procedural -2's always include a comment.

Whether adding a whole new vote type is a meaningful improvement is 
another question, but if we're adding the type anyway for prioritization 
it might make sense to use it to replace procedural -2. Especially if we 
could make it so any core can change it (apparently not right now), 
whereas -2 requires the original core to come back and remove it.