[Development] [RFC] more gerrit codereview scores?

Thiago Macieira thiago.macieira at intel.com
Sat Mar 7 04:03:30 CET 2015


On Friday 06 March 2015 17:42:00 Oswald Buddenhagen wrote:
> 1) i'd like to propose the introduction of the code review score -3.

> -1: "I would prefer this is not merged as is", advisory, non-sticky
> -2: "This shall not be merged as is", blocking, non-sticky
> -3: "This shall not be merged [at all]", blocking, sticky

Agreed, this makes sense. The -1 means "if someone approves, ignore me", 
whereas -2 means "this needs to change, can't be merged".

The -3 is just to indicate an approach that can never work, such as a feature 
we cannot accept or a patch submitted to the wrong branch. -2 are already 
rather rare, so I expect -3 to be used only once in a blue moon.

> 2) i'd like to propose the introduction of the code review score +3.
> 
> let's start with the scores:
> 
> +3: "Looks good to me, approved", enabling
> +2: "Looks good to me, but someone else must approve", advisory
> +1: "Someone else must review this", advisory
> 
> possible uses:
> - non-approvers (specifically, not-yet-approvers) would have two levels
>   to express their opinion

You mean four levels, since they also get -1 and 0.

> - the new +1 gives the possibility to explicitly give a neutral score
>   (substitute for +0, which gerrit does not permit)
> - *maybe* some approvers would feel less inclined to approve changes
>   they don't fully understand (yes, this is actually a problem), simply
>   because of the psychological effect of the possibility to express the
>   opinion with more "numerical nuance".
> 
> i don't feel very strongly about this one, but i think it would add
> value.

I don't like this one.

If you don't want to express an opinion, leave your score at 0. I don't see 
the need to have a value saying "I've reviewed but have no opinion". I also 
don't see why approvers are giving +2 if they don't fully understand it. Not 
only should they be using the right value for that, this change wouldn't help 
in any way since they could just turn around and give +3 to changes they don't 
fully understand.

As a drawback, it would make Qt's Gerrit behave very differently from everyone 
else, where a +2 does mean approval.

In my opinion, this change has no pros and has cons.

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center




More information about the Development mailing list