[Development] [RFC] more gerrit codereview scores?
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
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
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