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 _______________________________________________ Development mailing list Development@qt-project.org http://lists.qt-project.org/mailman/listinfo/development
