[Development] [RFC] more gerrit codereview scores?

Konstantin Ritt ritt.ks at gmail.com
Thu Mar 19 04:10:17 CET 2015


[Not really in-topic but still...]
Can we also introduce a flag with meaning like "this change doesn't require
clean build"?
For example, when the approver gives his +2 to a simple change in the .h
file, he can also turn that flag on -- iff all staged changes has this flag
on, then CI does incremental build and runs auto-tests as usual.


Konstantin

2015-03-09 11:30 GMT+04:00 Knoll Lars <Lars.Knoll at theqtcompany.com>:

> On 07/03/15 04:03, "Thiago Macieira" <thiago.macieira at intel.com> wrote:
>
> >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.
>
> Agree, it would be nice to have a non sticky -2, so the sticky -3 makes
> sense.
> >
> >> 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.
>
> While I see the reasoning behind, I think this overcomplicates things. A
> non-sticky -2 that balances a +2 should be enough to solve most of the
> issues, and the proposed +1 sounds very much like the current +0 to me, so
> we can IMO just as well leave it out.
>
> Cheers,
> Lars
>
> _______________________________________________
> Development mailing list
> Development at qt-project.org
> http://lists.qt-project.org/mailman/listinfo/development
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.qt-project.org/pipermail/development/attachments/20150319/bff96db7/attachment.html>


More information about the Development mailing list