Review-Priority for Project Repos

Erik Olof Gunnar Andersson eandersson at blizzard.com
Fri Jan 11 08:13:41 UTC 2019


This has worked great for Designate as most of the reviewers have limited time, it has helped us focus on core issues and get critical patches out a lot faster than we otherwise would.

Sent from my iPhone

> On Jan 10, 2019, at 9:14 AM, Ben Nemec <openstack at nemebean.com> wrote:
> 
> 
> 
>> 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.
> 



More information about the openstack-discuss mailing list