----- Original Message ----- > From: "Ryan Harper" <ry...@us.ibm.com> > To: "Dan Kenigsberg" <dan...@redhat.com> > Cc: "Alon Bar-Lev" <alo...@redhat.com>, "Doron Fediuck" > <dfedi...@redhat.com>, "Mark Wu" <wu...@linux.vnet.ibm.com>, > "Greg Padgett" <gpadg...@redhat.com>, vdsm-devel@lists.fedorahosted.org, > "Ryan Harper" <ry...@us.ibm.com>, "Ayal > Baron" <aba...@redhat.com> > Sent: Tuesday, October 2, 2012 4:32:30 PM > Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache' instead of > 'yum clean all' > > * Dan Kenigsberg <dan...@redhat.com> [2012-10-02 09:03]: > > On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote: > > > > > > > > > ----- Original Message ----- > > > > From: "Dan Kenigsberg" <dan...@redhat.com> > > > > To: "Alon Bar-Lev" <alo...@redhat.com> > > > > Cc: "Doron Fediuck" <dfedi...@redhat.com>, "Mark Wu" > > > > <wu...@linux.vnet.ibm.com>, "Greg Padgett" > > > > <gpadg...@redhat.com>, vdsm-devel@lists.fedorahosted.org, "Ryan > > > > Harper" <ry...@us.ibm.com>, "Ayal Baron" > > > > <aba...@redhat.com> > > > > Sent: Tuesday, October 2, 2012 3:26:31 PM > > > > Subject: Re: Change in vdsm[master]: Use 'yum clean > > > > expire-cache' instead of 'yum clean all' > > > > > > > > On Tue, Oct 02, 2012 at 08:59:05AM -0400, Alon Bar-Lev wrote: > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > From: "Doron Fediuck" <dfedi...@redhat.com> > > > > > > To: "Alon Bar-Lev" <alo...@redhat.com> > > > > > > Cc: "Mark Wu" <wu...@linux.vnet.ibm.com>, "Dan Kenigsberg" > > > > > > <dan...@redhat.com>, "Greg Padgett" <gpadg...@redhat.com>, > > > > > > vdsm-devel@lists.fedorahosted.org, "Ryan Harper" > > > > > > <ry...@us.ibm.com>, "Ayal Baron" <aba...@redhat.com> > > > > > > Sent: Tuesday, October 2, 2012 2:28:07 PM > > > > > > Subject: Re: Change in vdsm[master]: Use 'yum clean > > > > > > expire-cache' > > > > > > instead of 'yum clean all' > > > > > > > > > > > > ----- Original Message ----- > > > > > > > From: "Alon Bar-Lev" <alo...@redhat.com> > > > > > > > To: "Ryan Harper" <ry...@us.ibm.com> > > > > > > > Cc: "Mark Wu" <wu...@linux.vnet.ibm.com>, "Dan > > > > > > > Kenigsberg" > > > > > > > <dan...@redhat.com>, "Greg Padgett" > > > > > > > <gpadg...@redhat.com>, > > > > > > > "Doron Fediuck" <dfedi...@redhat.com>, > > > > > > > vdsm-devel@lists.fedorahosted.org > > > > > > > Sent: Monday, October 1, 2012 10:53:31 PM > > > > > > > Subject: Re: Change in vdsm[master]: Use 'yum clean > > > > > > > expire-cache' > > > > > > > instead of 'yum clean all' > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----- Original Message ----- > > > > > > > > From: "Ryan Harper" <ry...@us.ibm.com> > > > > > > > > To: vdsm-devel@lists.fedorahosted.org > > > > > > > > Cc: "Mark Wu" <wu...@linux.vnet.ibm.com>, "Dan > > > > > > > > Kenigsberg" > > > > > > > > <dan...@redhat.com>, "Greg Padgett" > > > > > > > > <gpadg...@redhat.com>, > > > > > > > > "Doron Fediuck" <dfedi...@redhat.com>, "Alon Bar-Lev" > > > > > > > > <alo...@redhat.com> > > > > > > > > Sent: Monday, October 1, 2012 10:24:08 PM > > > > > > > > Subject: Re: Change in vdsm[master]: Use 'yum clean > > > > > > > > expire-cache' > > > > > > > > instead of 'yum clean all' > > > > > > > > > > > > > > > > * Alon Bar-Lev <alo...@redhat.com> [2012-09-27 13:38]: > > > > > > > > > Alon Bar-Lev has posted comments on this change. > > > > > > > > > > > > > > > > > > Change subject: Use 'yum clean expire-cache' instead > > > > > > > > > of > > > > > > > > > 'yum > > > > > > > > > clean > > > > > > > > > all' > > > > > > > > > ...................................................................... > > > > > > > > > > > > > > > > > > > > > > > > > > > Patch Set 2: > > > > > > > > > > > > > > > > > > Ok... I was discussing... I think that if you don't > > > > > > > > > get +1 > > > > > > > > > from > > > > > > > > > parties you should wait... :) > > > > > > > > > > > > > > > > > > I see -1 as final decision... for the entire > > > > > > > > > change... or > > > > > > > > > if > > > > > > > > > contributer is not cooperating. > > > > > > > > > > > > > > > > > > > > > > > > > I'm interested in a little clarity here. > > > > > > > > > > > > > > > > As I see it, -1 means you don't want the current > > > > > > > > version > > > > > > > > submitted. > > > > > > > > > > > > > > > > I like the idea of putting a patch on hold while > > > > > > > > various > > > > > > > > issues > > > > > > > > are > > > > > > > > discussed, and it seems like a -1 is the right idea > > > > > > > > here > > > > > > > > since > > > > > > > > the > > > > > > > > submitter can reply and original reviewer can re-review > > > > > > > > and > > > > > > > > remove > > > > > > > > a > > > > > > > > -1 > > > > > > > > if the submitter has fully explained the issue. > > > > > > > > Additionaly > > > > > > > > the > > > > > > > > submitter can resubmit with changes (and the -1 is > > > > > > > > removed > > > > > > > > anyhow). > > > > > > > > > > > > > > This is exactly the problem... you cannot rely on -1 as > > > > > > > it > > > > > > > clears > > > > > > > if > > > > > > > a new patchset is pushed. > > > > > > > > At the moment, the job of the maintainer cannot be done by a > > > > script. > > > > The > > > > maintainer has to review former opinions on the patch, and > > > > check if > > > > they > > > > have been addressed. If a valuable reviewer gave an opinionated > > > > -1, > > > > and > > > > it was not addressed in a later version, the mainatainer should > > > > not > > > > take > > > > the patch. > > > > > > > > To me, "-1" means: "hey, Dan, please do not take this patch > > > > into > > > > master > > > > before we get an answer to my worries, unless there is a more > > > > urgent > > > > reason to take the patch earlier". > > > > > > Hi Dan, > > > > > > I don't understand why you don't treat "0" at the above... > > > > > > If there were no worries, +1 had been provided... > > > > To me, "0" means "I do not have a strong opinion, I trust other > > people > > to make the right decision, given the facts and worries that I have > > raised". > > > > Sometimes I do not give a +1 simply because I did not have time to > > review > > the whole code, not because I have a strong worry. "0" means "not > > reviewed yet" or "not endorsed yet by me but not rejected by me". > > > > A polite and well-detailed -1 should be used daily and not > > considered > > "rude". > > Agreed. It took a little getting used to for myself (mostly because > of > the 'I would prefer you didn't submit this'), but I think a -1 > with comments is much more valuable than a 0 with comments. If I > leave a > 0 with comments, I feel that I'm telling the maintainer that I don't > feel strongly enough to force the submitter to change the code. > > Using -1 or +1 is similar to Ack and Nack, which give an affirmative > yes or > no. Should this patch be merged in its current form? That's a > binary > question, hence the need for a +1 or -1.
I disagree in that regard. There is a huge difference between discussion and rejection. Having the scale of -1, 0, +1 and not using 0 for discussion actually narrow the scale by 30% to -1, +1. ACK - is a complete ACK to be merge "If I was the maintainer I would have submitted". NAK - is a complete NACK "If I was the maintainer I would have rejected". ACK/NAK are methods of decisions for the entire patch, not for iterative discussion, nor for bookmark what I've reviewed. Maybe in future the scale will be larger... -3...+3, so unprivileged user get -2..+2, then minor comments can be at -1..+1 range, but we are not there. I don't like we narrow the scale in vdsm in 30%... Alon. _______________________________________________ vdsm-devel mailing list vdsm-devel@lists.fedorahosted.org https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
