[Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself. Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar. Thanks, Ofer Schreiber
On 19/02/13 11:51, Ofer Schreiber wrote:
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
I think 'I would prefer that you didn't' is fine, I'm not sure why 'submit this' and not 'merge this'. Y.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On Tue, Feb 19, 2013 at 11:54:56AM +0200, Yaniv Kaul wrote:
On 19/02/13 11:51, Ofer Schreiber wrote:
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
I think 'I would prefer that you didn't' is fine, I'm not sure why 'submit this' and not 'merge this'.
Gerrit uses the "submit" term for "taking a change into a branch" elsewhere, too. Maybe because "merge" is something else in gittish.
Y.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
I don't care much about the text, but many people told me that "I would prefer that you didn't submit this" sounds condensding. So a change is in place, and I do not mind your suggestion, Ofer. Dan.
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. -2: I disagree. ----- Original Message -----
From: "Ofer Schreiber" <oschreib@redhat.com> To: engine-devel@ovirt.org Sent: Tuesday, February 19, 2013 10:51:15 AM Subject: [Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
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"
-2: I disagree.
"-2: Please reconsider"
----- Original Message -----
From: "Ofer Schreiber" <oschreib@redhat.com> To: engine-devel@ovirt.org Sent: Tuesday, February 19, 2013 10:51:15 AM Subject: [Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
I like most of these proposals. It'll make gerrit friendlier :-) ----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Laszlo Hornyak" <lhornyak@redhat.com> Cc: engine-devel@ovirt.org Sent: Wednesday, February 20, 2013 9:32:53 AM Subject: Re: [Engine-devel] Changing Gerrit -1 message
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"
-2: I disagree.
"-2: Please reconsider"
----- Original Message -----
From: "Ofer Schreiber" <oschreib@redhat.com> To: engine-devel@ovirt.org Sent: Tuesday, February 19, 2013 10:51:15 AM Subject: [Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
I tend to agree as well regarding the -1 message, although I think that since -2 blocks your change from, merging it, it should still be strict. Perhaps we could consider adding one more negative value? Regards, Maor On 02/20/2013 11:59 AM, Antoni Segura Puimedon wrote:
I like most of these proposals. It'll make gerrit friendlier :-)
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Laszlo Hornyak" <lhornyak@redhat.com> Cc: engine-devel@ovirt.org Sent: Wednesday, February 20, 2013 9:32:53 AM Subject: Re: [Engine-devel] Changing Gerrit -1 message
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"
-2: I disagree.
"-2: Please reconsider"
----- Original Message -----
From: "Ofer Schreiber" <oschreib@redhat.com> To: engine-devel@ovirt.org Sent: Tuesday, February 19, 2013 10:51:15 AM Subject: [Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
----- Original Message -----
From: "Maor Lipchuk" <mlipchuk@redhat.com> To: engine-devel@ovirt.org Sent: Thursday, February 21, 2013 12:25:57 AM Subject: Re: [Engine-devel] Changing Gerrit -1 message
I tend to agree as well regarding the -1 message, although I think that since -2 blocks your change from, merging it, it should still be strict. Perhaps we could consider adding one more negative value?
In my opinion, adding another negative value isn't so friendly... we should be more positive and not negative :-) I liked the "-1: Please review my comments" proposed below. I think the current "-2: do not submit" is pretty polite .... If you guys think it requires a change, then I'd change it to: "-2: do not submit. Please review my comments." Oved
Regards, Maor
On 02/20/2013 11:59 AM, Antoni Segura Puimedon wrote:
I like most of these proposals. It'll make gerrit friendlier :-)
----- Original Message -----
From: "Itamar Heim" <iheim@redhat.com> To: "Laszlo Hornyak" <lhornyak@redhat.com> Cc: engine-devel@ovirt.org Sent: Wednesday, February 20, 2013 9:32:53 AM Subject: Re: [Engine-devel] Changing Gerrit -1 message
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"
-2: I disagree.
"-2: Please reconsider"
----- Original Message -----
From: "Ofer Schreiber" <oschreib@redhat.com> To: engine-devel@ovirt.org Sent: Tuesday, February 19, 2013 10:51:15 AM Subject: [Engine-devel] Changing Gerrit -1 message
I feel that the current "-1 I would prefer that you didn't submit this" message in Gerrit is pretty rude, as usually those -1 reviews are just small fix-ups in the code itself.
Any thoughts about a more suitable "-1" message? I thought about "-1 Please fix your code" or something similar.
Thanks, Ofer Schreiber
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
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"
Sounds great.
-2: I disagree.
"-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. -- Dave Neary - Community Action and Impact Open Source and Standards, Red Hat - http://community.redhat.com Ph: +33 9 50 71 55 62 / Cell: +33 6 77 01 92 13
----- Original Message -----
From: "Dave Neary" <dneary@redhat.com> To: engine-devel@ovirt.org 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
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
"-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.
-- Dave Neary - Community Action and Impact Open Source and Standards, Red Hat - http://community.redhat.com Ph: +33 9 50 71 55 62 / Cell: +33 6 77 01 92 13 _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
On 02/22/2013 12:53 AM, Eli Mesika wrote:
----- Original Message -----
From: "Dave Neary" <dneary@redhat.com> To: engine-devel@ovirt.org 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
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: 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.
-- Dave Neary - Community Action and Impact Open Source and Standards, Red Hat - http://community.redhat.com Ph: +33 9 50 71 55 62 / Cell: +33 6 77 01 92 13 _______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
_______________________________________________ Engine-devel mailing list Engine-devel@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-devel
----- Original Message -----
From: "Moti Asayag" <masayag@redhat.com> To: "Eli Mesika" <emesika@redhat.com> Cc: engine-devel@ovirt.org 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" <dneary@redhat.com> To: engine-devel@ovirt.org 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.
participants (12)
-
Antoni Segura Puimedon -
Dan Kenigsberg -
Dave Neary -
Doron Fediuck -
Eli Mesika -
Itamar Heim -
Laszlo Hornyak -
Maor Lipchuk -
Moti Asayag -
Ofer Schreiber -
Oved Ourfalli -
Yaniv Kaul