----- Original Message ----- > From: "Moti Asayag" <[email protected]> > To: "Eli Mesika" <[email protected]> > Cc: [email protected] > Sent: Friday, February 22, 2013 9:02:08 AM > Subject: Re: [Engine-devel] Changing Gerrit -1 message > > On 02/22/2013 12:53 AM, Eli Mesika wrote: > > > > > > ----- Original Message ----- > >> From: "Dave Neary" <[email protected]> > >> To: [email protected] > >> Sent: Thursday, February 21, 2013 11:21:53 PM > >> Subject: Re: [Engine-devel] Changing Gerrit -1 message > >> > >> Hi, > >> > >> On 02/20/2013 10:32 AM, Itamar Heim wrote: > >>> On 19/02/2013 12:06, Laszlo Hornyak wrote: > >>>> Hi, > >>>> > >>>> I agree with that. > >>>> > >>>> for the - messages this in my opinion would be both more clear > >>>> and > >>>> friendly: > >>>> -1: In my opinion it needs work. > >>> > >>> > >>> how about > >>> "-1: Please review my comments" > > > > +1 > > +1 >
Seems that we're in agreement on this one.... > > > >> > >> Sounds great. > >> > >>>> -2: I disagree. > > > > I prefer -2 : Do not submit! > > > > IMHO: > > -1 should be used whenever the code is OK but can be done better or > > has a missing part (tests for example) > > -2 should be used when the code does not work, has a serious bug > > (possible NPE for example) , break the build > > +1 for the -2 which to my opinion should indicate any serious flaw in > the design, even post the design discussion. I think that this should > follow with a discussion on engine-devel about the required change in > the design instead of debating over the gerrit since it is not > transparent enough. > -2 should be given on rare occasions, as it is so blunt and meaningful. Remember that -1 is good enough to block a patch. So if you see an issue which may be fixed, give it -1. If you see something which is illegal (violate project license) or contradicts the project foundations, give it -2. This is how rare -2 should be. In this view I'd rephrase "-2" to: "This patch can not be accepted". Obviously this should be followed by a detailed explanation by the reviewer. > > > >>> > >>> "-2: Please reconsider" > >> > >> I am trying to think under which circumstances people give a -2. > >> Maybe > >> something like "We have discussed this feature, and I disagree > >> that > >> it > >> is good for the project." Basically, I don't think that a > >> developer > >> should ever see a -2, unless they specifically disagree with the > >> maintainer, and insist that they are right, to the point of > >> repeatedly > >> submitting patches. > >> > >> Cheers, > >> Dave. > >> _______________________________________________ Engine-devel mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-devel
