[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.kn...@theqtcompany.com>: > On 07/03/15 04:03, "Thiago Macieira" <thiago.macie...@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@qt-project.org > http://lists.qt-project.org/mailman/listinfo/development >
_______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
