Open Stack

On Thu, 2019-01-10 at 13:42 -0600, Sean McGinnis wrote:
> > 
> > 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.
> > 
> 
> This was exactly the reasoning. -2 is overloaded, but its primary meaning
> was/is "we do not want this code change". It just happens that it was also a
> convenient way to say that with "right now" at the end.
> 
> The Review-Priority -1 is a clear way to say whether something is held because
> it can't be merged right now due to procedural or process reasons, versus
> something that we just don't want at all.
for what its worth my understanding of why a procdural -2 is more correct is that this change 
cannot be merged because it has not met the procedual requirement to be considerd for this release.
haveing received several over the years i have never seen it to carry any malaise 
or weight then the zuul pep8 job complianing about the line lenght of my code.
with either a procedural -2  or a verify -1 from zuul my code is equally un mergeable.

the prime example being a patch that requires a spec that has not been approved.
while most cores will not approve chage when other cores have left a -1 mistakes happen
and the -2 does emphasise the point that even if the code is perfect under the porject
processes this change should not be acitvly reporposed until the issue raised by the -2
has been addressed.  In the case of a procedual -2 that typically means the spec is merge
or the master branch opens for the next cycle.

i agree that procedural -2's can seam harsh at first glance but i have also never seen one
left without a comment explaining why it was left. the issue with a procedural -1 is i can 
jsut resubmit the patch several times and it can get lost in the comments.

we recently intoduced a new review priority lable
if we really wanted to disabiguate form normal -2s then we coudl have an explcitly lable for it
but i personally would prefer to keep procedural -2s.

anyway that just my two cents.
> 
>