[Development] [RFC] more gerrit codereview scores?

Knoll Lars Lars.Knoll at theqtcompany.com
Thu Mar 19 08:35:47 CET 2015


Not currently. It just doesn't work in the current CI system. But we're currently working on a new and much improved CI system (more info soon in a separate mail), that should at least improve the long integration times.

Cheers,
Lars

On 19/03/15 04:10, "Konstantin Ritt" <ritt.ks at gmail.com<mailto:ritt.ks at gmail.com>> wrote:

[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<mailto:Lars.Knoll at theqtcompany.com>>:
On 07/03/15 04:03, "Thiago Macieira" <thiago.macieira at intel.com<mailto: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<mailto: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/995d84a3/attachment.html>


More information about the Development mailing list