3:24 p.m.
* Alon Bar-Lev <alonbl(a)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).
What is the maintainer view of how best to use -1? And if we don't use
a -1, how does one do the above?
--
To view, visit http://gerrit.ovirt.org/8206
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I1019690e4a27caeff923655f9562892ce7f909af
Gerrit-PatchSet: 2
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Mark Wu <wudxw(a)linux.vnet.ibm.com>
Gerrit-Reviewer: Alon Bar-Lev <alonbl(a)redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken(a)redhat.com>
Gerrit-Reviewer: Doron Fediuck <dfediuck(a)redhat.com>
Gerrit-Reviewer: Douglas Schilling Landgraf <dougsland(a)redhat.com>
Gerrit-Reviewer: Greg Padgett <gpadgett(a)redhat.com>
Gerrit-Reviewer: Mark Wu <wudxw(a)linux.vnet.ibm.com>
_______________________________________________
vdsm-patches mailing list
vdsm-patches(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
3:53 p.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Ryan Harper" <ryanh(a)us.ibm.com>
To: vdsm-devel(a)lists.fedorahosted.org
Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan Kenigsberg"
<danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
"Doron Fediuck" <dfediuck(a)redhat.com>, "Alon Bar-Lev"
<alonbl(a)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 <alonbl(a)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.
Only +1 is to be considered before merge.
What is the maintainer view of how best to use -1? And if we
don't
use
a -1, how does one do the above?
I think that this is fairly new to have proper procedure set.
There are people who use -1 as labelling of what they reviewed.
There are people who use -1 if they have any comment.
There are people who use -1 if they feel patch (the idea, mean and description) should not
be submitted.
My view is that among the -2, -1, 0, +1, +2 there is a proper room for 0, 0 is a number
just like any other number, removing it from scale means narrowing the scale.
My personal view is that:
0 - discussion taking place (initial state).
+1 - approved for submission by reviewer.
+2 - approve for submission by maintainer.
-1 - either the change (principal) is rejected, or the patch is so bad that need a major
rework, or contributor is not cooperating.
-2 - final rejection by maintainer.
But this is only my view...
BTW: it works fine at engine patches... all discussion takes place at '0' until a
decision of -1 or +1 and then final decision of -2 or +2.
Regards,
Alon.
7:28 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Alon Bar-Lev" <alonbl(a)redhat.com>
To: "Ryan Harper" <ryanh(a)us.ibm.com>
Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan Kenigsberg"
<danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
"Doron Fediuck" <dfediuck(a)redhat.com>, vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> To: vdsm-devel(a)lists.fedorahosted.org
> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> <danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
> "Doron Fediuck" <dfediuck(a)redhat.com>, "Alon Bar-Lev"
> <alonbl(a)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 <alonbl(a)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.
Only +1 is to be considered before merge.
> What is the maintainer view of how best to use -1? And if we don't
> use
> a -1, how does one do the above?
I think that this is fairly new to have proper procedure set.
There are people who use -1 as labelling of what they reviewed.
There are people who use -1 if they have any comment.
There are people who use -1 if they feel patch (the idea, mean and
description) should not be submitted.
My view is that among the -2, -1, 0, +1, +2 there is a proper room
for 0, 0 is a number just like any other number, removing it from
scale means narrowing the scale.
My personal view is that:
0 - discussion taking place (initial state).
+1 - approved for submission by reviewer.
+2 - approve for submission by maintainer.
-1 - either the change (principal) is rejected, or the patch is so
bad that need a major rework, or contributor is not cooperating.
-2 - final rejection by maintainer.
But this is only my view...
BTW: it works fine at engine patches... all discussion takes place at
'0' until a decision of -1 or +1 and then final decision of -2 or
+2.
Regards,
Alon.
FWIW, all marks are reset when a patch is rebased.
This does /not/ mean we can ignore patch history (unless technical issues such as
Jenkins).
As for the meaning of the numbers this is my general policy for all projects,
which I know many maintainers share. Alon's view is close in some points;
(-2) This change is either:
- Malicious, Illegal or in some other violation
- Breaks compilation, build or design
That's it. This grade is blunt. Do not use it for any other case.
(-1) Any other case where you need to block the patch.
(0) Any comment you wish to add. This may include questions and suggestions.
Use this and do not block the change if there's no *real* reason to block it.
(1) The patch works, but...
- I cannot assure it correctness system-wide.
- I'd like to reserve what I'm +1-ing for by adding a general comment (e.g.
"+1 for style, did not review correctness)
- Or "Flow looks good to me, unsure about style"
- Or "Looks correct but risky so needs second opinion" (note that in storage the
norm is to have at least 2 reviewers ack before merging).
(2) The patch works. To my best knowledge, it's correct system-wide.
And I understand the subject materia and I am willing to sign my name on this patch.
For storage- this patch already has at least 1 additional +1 about correctness, not
style.
Examples (search Gerrit for me as a reviewer);
(-2)
- Including a dependency on some package with bad licensing.
(-1)
- A patch which may break integrations with ovirt node and needs verification
(0)
- need info on something, comments for future readers, etc.
(+1)
- All vdsm patches I approve
- Engine patches related to UI / storage / networking
- Asking for improvments
(+2)
- Many infra / all SLA related patches
- All other patches which I fully understand and can vouch for.
7:59 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Doron Fediuck" <dfediuck(a)redhat.com>
To: "Alon Bar-Lev" <alonbl(a)redhat.com>
Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan Kenigsberg"
<danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper" <ryanh(a)us.ibm.com>,
"Ayal Baron" <abaron(a)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" <alonbl(a)redhat.com>
> To: "Ryan Harper" <ryanh(a)us.ibm.com>
> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> <danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
> "Doron Fediuck" <dfediuck(a)redhat.com>,
> vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > To: vdsm-devel(a)lists.fedorahosted.org
> > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > "Doron Fediuck" <dfediuck(a)redhat.com>, "Alon
Bar-Lev"
> > <alonbl(a)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 <alonbl(a)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.
>
> Only +1 is to be considered before merge.
>
> > What is the maintainer view of how best to use -1? And if we
> > don't
> > use
> > a -1, how does one do the above?
>
> I think that this is fairly new to have proper procedure set.
>
> There are people who use -1 as labelling of what they reviewed.
> There are people who use -1 if they have any comment.
> There are people who use -1 if they feel patch (the idea, mean and
> description) should not be submitted.
>
> My view is that among the -2, -1, 0, +1, +2 there is a proper room
> for 0, 0 is a number just like any other number, removing it from
> scale means narrowing the scale.
>
> My personal view is that:
>
> 0 - discussion taking place (initial state).
> +1 - approved for submission by reviewer.
> +2 - approve for submission by maintainer.
> -1 - either the change (principal) is rejected, or the patch is so
> bad that need a major rework, or contributor is not cooperating.
> -2 - final rejection by maintainer.
>
> But this is only my view...
>
> BTW: it works fine at engine patches... all discussion takes place
> at
> '0' until a decision of -1 or +1 and then final decision of -2 or
> +2.
>
> Regards,
> Alon.
>
FWIW, all marks are reset when a patch is rebased.
This does /not/ mean we can ignore patch history (unless technical
issues such as Jenkins).
As for the meaning of the numbers this is my general policy for all
projects,
which I know many maintainers share. Alon's view is close in some
points;
(-2) This change is either:
- Malicious, Illegal or in some other violation
- Breaks compilation, build or design
That's it. This grade is blunt. Do not use it for any other case.
(-1) Any other case where you need to block the patch.
(0) Any comment you wish to add. This may include questions and
suggestions.
Use this and do not block the change if there's no *real* reason to
block it.
(1) The patch works, but...
- I cannot assure it correctness system-wide.
- I'd like to reserve what I'm +1-ing for by adding a general comment
(e.g. "+1 for style, did not review correctness)
- Or "Flow looks good to me, unsure about style"
- Or "Looks correct but risky so needs second opinion" (note that in
storage the norm is to have at least 2 reviewers ack before
merging).
I agree with most but not with this one...
Keep in mind that +1 is available for all people while +2 is not.
So you cannot abuse +1 for conceptual notes.
+1 should be a statement like:
"Had I been the maintainer I would have submitted this change"
"I am not the maintainer nor do I have +2 privilege, so I here is my +1"
Of course one can note that he only capable of reviewing X among the entire change.
For example DBA review the database changes.
Then maintainer reintegrate the collective +1s into a complete +2.
(2) The patch works. To my best knowledge, it's correct
system-wide.
And I understand the subject materia and I am willing to sign my name
on this patch.
For storage- this patch already has at least 1 additional +1 about
correctness, not style.
This would have been true if all had +2.
Thanks!
Examples (search Gerrit for me as a reviewer);
(-2)
- Including a dependency on some package with bad licensing.
(-1)
- A patch which may break integrations with ovirt node and needs
verification
(0)
- need info on something, comments for future readers, etc.
(+1)
- All vdsm patches I approve
- Engine patches related to UI / storage / networking
- Asking for improvments
(+2)
- Many infra / all SLA related patches
- All other patches which I fully understand and can vouch for.
8:26 a.m.
New subject: 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" <dfediuck(a)redhat.com>
> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan Kenigsberg"
<danken(a)redhat.com>, "Greg Padgett" <gpadgett(a)redhat.com>,
> vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper" <ryanh(a)us.ibm.com>,
"Ayal Baron" <abaron(a)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" <alonbl(a)redhat.com>
> > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > > To: vdsm-devel(a)lists.fedorahosted.org
> > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > "Doron Fediuck" <dfediuck(a)redhat.com>, "Alon
Bar-Lev"
> > > <alonbl(a)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 <alonbl(a)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".
Dan.
8:34 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Dan Kenigsberg" <danken(a)redhat.com>
To: "Alon Bar-Lev" <alonbl(a)redhat.com>
Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
<gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
<ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <dfediuck(a)redhat.com>
> > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > <ryanh(a)us.ibm.com>, "Ayal Baron" <abaron(a)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" <alonbl(a)redhat.com>
> > > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > > vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > > > To: vdsm-devel(a)lists.fedorahosted.org
> > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > "Doron Fediuck" <dfediuck(a)redhat.com>, "Alon
Bar-Lev"
> > > > <alonbl(a)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 <alonbl(a)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...
Alon.
9:02 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote:
----- Original Message -----
> From: "Dan Kenigsberg" <danken(a)redhat.com>
> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
> <abaron(a)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" <dfediuck(a)redhat.com>
> > > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > > <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
> > > > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > > > vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > > > > To: vdsm-devel(a)lists.fedorahosted.org
> > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
> > > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
> > > > > <alonbl(a)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 <alonbl(a)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".
Dan.
9:32 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
* Dan Kenigsberg <danken(a)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" <danken(a)redhat.com>
> > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
> > <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
> > <abaron(a)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" <dfediuck(a)redhat.com>
> > > > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > > > <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
> > > > > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
> > > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > > > > vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > > > > > To: vdsm-devel(a)lists.fedorahosted.org
> > > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
> > > > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
> > > > > > <alonbl(a)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 <alonbl(a)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.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
9:41 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Ryan Harper" <ryanh(a)us.ibm.com>
To: "Dan Kenigsberg" <danken(a)redhat.com>
Cc: "Alon Bar-Lev" <alonbl(a)redhat.com>, "Doron Fediuck"
<dfediuck(a)redhat.com>, "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Greg Padgett" <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org,
"Ryan Harper" <ryanh(a)us.ibm.com>, "Ayal
Baron" <abaron(a)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 <danken(a)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" <danken(a)redhat.com>
> > > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark
Wu"
> > > <wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
> > > <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org,
"Ryan
> > > Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
> > > <abaron(a)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" <dfediuck(a)redhat.com>
> > > > > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
> > > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > > > > <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
> > > > > > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan
> > > > > > Kenigsberg"
> > > > > > <danken(a)redhat.com>, "Greg Padgett"
> > > > > > <gpadgett(a)redhat.com>,
> > > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > > > > > vdsm-devel(a)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"
<ryanh(a)us.ibm.com>
> > > > > > > To: vdsm-devel(a)lists.fedorahosted.org
> > > > > > > Cc: "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Dan
> > > > > > > Kenigsberg"
> > > > > > > <danken(a)redhat.com>, "Greg Padgett"
> > > > > > > <gpadgett(a)redhat.com>,
> > > > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
> > > > > > > <alonbl(a)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 <alonbl(a)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.
10:06 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
* Alon Bar-Lev <alonbl(a)redhat.com> [2012-10-02 09:43]:
> From: "Ryan Harper" <ryanh(a)us.ibm.com>
> To: "Dan Kenigsberg" <danken(a)redhat.com>
> Cc: "Alon Bar-Lev" <alonbl(a)redhat.com>, "Doron Fediuck"
<dfediuck(a)redhat.com>, "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
> "Greg Padgett" <gpadgett(a)redhat.com>,
vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper" <ryanh(a)us.ibm.com>,
"Ayal
> Baron" <abaron(a)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'
>
> > >
> > > 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%...
I don't care that much for a scale; the numbers are arbitrary IMO.
I just need a way to ensure that the maintainer (and the submitter)
understand that that IMO, the current patch isn't ready for acceptance.
And that the objection to submission in the current state isn't lost.
If I provide review comments, but don't include a -1, it wasn't clear to
me that the maintainer would view this comment as a real concern. A -1
plus constructive review comment IMO makes this point clearly to the
maintainer.
Dan has clarified his view on the scale; and as long as I understand how
it's being used; that's good enough for me.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
10:24 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Ryan Harper" <ryanh(a)us.ibm.com>
To: "Alon Bar-Lev" <alonbl(a)redhat.com>
Cc: "Ryan Harper" <ryanh(a)us.ibm.com>, "Doron Fediuck"
<dfediuck(a)redhat.com>, "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Greg Padgett" <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org,
"Ayal Baron" <abaron(a)redhat.com>, "Dan
Kenigsberg" <danken(a)redhat.com>
Sent: Tuesday, October 2, 2012 5:06:28 PM
Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache' instead of
'yum clean all'
* Alon Bar-Lev <alonbl(a)redhat.com> [2012-10-02 09:43]:
> > From: "Ryan Harper" <ryanh(a)us.ibm.com>
> > To: "Dan Kenigsberg" <danken(a)redhat.com>
> > Cc: "Alon Bar-Lev" <alonbl(a)redhat.com>, "Doron
Fediuck"
> > <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>,
> > "Greg Padgett" <gpadgett(a)redhat.com>,
> > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > <ryanh(a)us.ibm.com>, "Ayal
> > Baron" <abaron(a)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'
> >
> > > >
> > > > 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%...
I don't care that much for a scale; the numbers are arbitrary IMO.
I just need a way to ensure that the maintainer (and the submitter)
understand that that IMO, the current patch isn't ready for
acceptance.
And that the objection to submission in the current state isn't lost.
If I provide review comments, but don't include a -1, it wasn't clear
to
me that the maintainer would view this comment as a real concern. A
-1
plus constructive review comment IMO makes this point clearly to the
maintainer.
Dan has clarified his view on the scale; and as long as I understand
how
it's being used; that's good enough for me.
It is just harder to work this way, this applies Dan as well who abuses the -1, and
applies only to vdsm among all the ovirt repositories.
I will repeat last time as I still unsure that the argument was clear enough, no need to
discuss this any farther.
You, as contributer that do not have -2, +2, have no mean to differentiate your view
between "I don't like this patch to be included, no matter what minor issues are
resolved", and "I find this patch to be useful and I work toward making it
perfect", when it is perfect I clearly state so using my +1 vote.
I find the differentiation between these cases extremely important. In time I guess we
learn the best approach, as we use one at vdsm and the other at all other projects. Of
course I think the other is more productive.
Regards,
Alon
10:34 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
* Alon Bar-Lev <alonbl(a)redhat.com> [2012-10-02 10:26]:
----- Original Message -----
> From: "Ryan Harper" <ryanh(a)us.ibm.com>
> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> Cc: "Ryan Harper" <ryanh(a)us.ibm.com>, "Doron Fediuck"
<dfediuck(a)redhat.com>, "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
> "Greg Padgett" <gpadgett(a)redhat.com>,
vdsm-devel(a)lists.fedorahosted.org, "Ayal Baron" <abaron(a)redhat.com>,
"Dan
> Kenigsberg" <danken(a)redhat.com>
> Sent: Tuesday, October 2, 2012 5:06:28 PM
> Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache' instead of
'yum clean all'
>
> * Alon Bar-Lev <alonbl(a)redhat.com> [2012-10-02 09:43]:
> > > From: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > To: "Dan Kenigsberg" <danken(a)redhat.com>
> > > Cc: "Alon Bar-Lev" <alonbl(a)redhat.com>, "Doron
Fediuck"
> > > <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>,
> > > "Greg Padgett" <gpadgett(a)redhat.com>,
> > > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > > <ryanh(a)us.ibm.com>, "Ayal
> > > Baron" <abaron(a)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'
> > >
> > > > >
> > > > > 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%...
>
> I don't care that much for a scale; the numbers are arbitrary IMO.
>
> I just need a way to ensure that the maintainer (and the submitter)
> understand that that IMO, the current patch isn't ready for
> acceptance.
> And that the objection to submission in the current state isn't lost.
> If I provide review comments, but don't include a -1, it wasn't clear
> to
> me that the maintainer would view this comment as a real concern. A
> -1
> plus constructive review comment IMO makes this point clearly to the
> maintainer.
>
> Dan has clarified his view on the scale; and as long as I understand
> how
> it's being used; that's good enough for me.
It is just harder to work this way, this applies Dan as well who
abuses the -1, and applies only to vdsm among all the ovirt
repositories.
I will repeat last time as I still unsure that the argument was clear
enough, no need to discuss this any farther.
You were perfectly clear; I just disagree.
You, as contributer that do not have -2, +2, have no mean to
differentiate your view between "I don't like this patch to be
included, no matter what minor issues are resolved", and "I find this
patch to be useful and I work toward making it perfect", when it is
perfect I clearly state so using my +1 vote.
I have no issue marking +1 when I'm happy with the patch. What I want
is way to raise a flag to the maintainer that I have a concern I would
like addressed before the patch is merged. If the maintainer chooses to
merge a patch even though a reviewer has marked -1, that's fine. But I
don't think just a 0 + comments is a strong enough position. Or at
least, I wanted clarification from the maintainers for their
preference.
If we didn't collapse a discussion into singular numbers, we wouldn't
wouldn't be discussing what a -1, 0, or +1 means. The words we've
written would matter more.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
9:35 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
From: "Dan Kenigsberg" <danken(a)redhat.com>
To: "Alon Bar-Lev" <alonbl(a)redhat.com>
Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
<gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
<ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)redhat.com>
Sent: Tuesday, October 2, 2012 4:02:44 PM
Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache' instead of
'yum clean all'
On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote:
>
>
> ----- Original Message -----
> > From: "Dan Kenigsberg" <danken(a)redhat.com>
> > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
> > <wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
> > <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
> > Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
> > <abaron(a)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" <dfediuck(a)redhat.com>
> > > > To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
> > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
> > > > <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
> > > > > To: "Ryan Harper" <ryanh(a)us.ibm.com>
> > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
> > > > > <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
> > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
> > > > > vdsm-devel(a)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" <ryanh(a)us.ibm.com>
> > > > > > To: vdsm-devel(a)lists.fedorahosted.org
> > > > > > Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan
> > > > > > Kenigsberg"
> > > > > > <danken(a)redhat.com>, "Greg Padgett"
> > > > > > <gpadgett(a)redhat.com>,
> > > > > > "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
> > > > > > <alonbl(a)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 <alonbl(a)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".
Dan.
Yes, this is the bookmark approach...
The problem is that most people has the -1, 0, +1 scale, and in that scale -1 is actually
the reject method.
"If I was the maintainer I would have rejected this patch".
Alon.
2:15 p.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
On 10/02/2012 04:35 PM, Alon Bar-Lev wrote:
----- Original Message -----
> From: "Dan Kenigsberg" <danken(a)redhat.com>
> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
> Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
> <abaron(a)redhat.com>
> Sent: Tuesday, October 2, 2012 4:02:44 PM
> Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache' instead of
'yum clean all'
>
> On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote:
>>
>>
>> ----- Original Message -----
>>> From: "Dan Kenigsberg" <danken(a)redhat.com>
>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>>> Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark
Wu"
>>> <wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
>>> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
>>> Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
>>> <abaron(a)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" <dfediuck(a)redhat.com>
>>>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>>>>> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
>>>>> <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
>>>>> vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
>>>>> <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
>>>>>> To: "Ryan Harper" <ryanh(a)us.ibm.com>
>>>>>> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
>>>>>> <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
>>>>>> "Doron Fediuck" <dfediuck(a)redhat.com>,
>>>>>> vdsm-devel(a)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" <ryanh(a)us.ibm.com>
>>>>>>> To: vdsm-devel(a)lists.fedorahosted.org
>>>>>>> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan
>>>>>>> Kenigsberg"
>>>>>>> <danken(a)redhat.com>, "Greg Padgett"
>>>>>>> <gpadgett(a)redhat.com>,
>>>>>>> "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
>>>>>>> <alonbl(a)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 <alonbl(a)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".
>
> Dan.
>
Yes, this is the bookmark approach...
The problem is that most people has the -1, 0, +1 scale, and in that scale -1 is actually
the reject method.
"If I was the maintainer I would have rejected this patch".
we could open -2 to non-maintainers as well if it is of use.
danken?
6:39 p.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
On 10/02/2012 04:35 PM, Alon Bar-Lev wrote:
>
>
> ----- Original Message -----
>> From: "Dan Kenigsberg" <danken(a)redhat.com>
>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>> Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark Wu"
>> <wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
>> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
>> Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
>> <abaron(a)redhat.com>
>> Sent: Tuesday, October 2, 2012 4:02:44 PM
>> Subject: Re: Change in vdsm[master]: Use 'yum clean expire-cache'
>> instead of 'yum clean all'
>>
>> On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote:
>>>
>>>
>>> ----- Original Message -----
>>>> From: "Dan Kenigsberg" <danken(a)redhat.com>
>>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>>>> Cc: "Doron Fediuck" <dfediuck(a)redhat.com>, "Mark
Wu"
>>>> <wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
>>>> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org,
"Ryan
>>>> Harper" <ryanh(a)us.ibm.com>, "Ayal Baron"
>>>> <abaron(a)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" <dfediuck(a)redhat.com>
>>>>>> To: "Alon Bar-Lev" <alonbl(a)redhat.com>
>>>>>> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
>>>>>> <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
>>>>>> vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
>>>>>> <ryanh(a)us.ibm.com>, "Ayal Baron"
<abaron(a)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" <alonbl(a)redhat.com>
>>>>>>> To: "Ryan Harper" <ryanh(a)us.ibm.com>
>>>>>>> Cc: "Mark Wu" <wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
>>>>>>> <danken(a)redhat.com>, "Greg Padgett"
<gpadgett(a)redhat.com>,
>>>>>>> "Doron Fediuck" <dfediuck(a)redhat.com>,
>>>>>>> vdsm-devel(a)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" <ryanh(a)us.ibm.com>
>>>>>>>> To: vdsm-devel(a)lists.fedorahosted.org
>>>>>>>> Cc: "Mark Wu"
<wudxw(a)linux.vnet.ibm.com>, "Dan
>>>>>>>> Kenigsberg"
>>>>>>>> <danken(a)redhat.com>, "Greg Padgett"
>>>>>>>> <gpadgett(a)redhat.com>,
>>>>>>>> "Doron Fediuck" <dfediuck(a)redhat.com>,
"Alon Bar-Lev"
>>>>>>>> <alonbl(a)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 <alonbl(a)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".
>>
>> Dan.
>>
>
> Yes, this is the bookmark approach...
>
> The problem is that most people has the -1, 0, +1 scale, and in
> that scale -1 is actually the reject method.
>
> "If I was the maintainer I would have rejected this patch".
we could open -2 to non-maintainers as well if it is of use.
danken?
Personally I disagree with this approach, -2 should rarely be given and doesn't
disappear. It should only be given to people who really understand the code properly and
whose opinion is well accepted by the community.
I agree with Alon that 0 can and should be used, I do not agree with him though that
ack/nack type of discussion should take place on it though and I'll explain why -
First of all, in gerrit there is no immediately visible difference between '0' and
no review at all so someone might have serious issues with a patch but if she did not mark
it with -1 submitter might totally miss this fact.
esp. if someone sent a new revision and the title of the cover comment for previous
version doesn't state a -1 (so maintainer doesn't know he needs to go looking back
to verify things were fixed).
This is adding overhead on maintainer now to go back to each and every review and make
sure that there are no comments that should have been addressed in 0.
Note that if someone gave a -1, normally I'd expect that person to make sure and +1 a
subsequent patch to flag to maintainer that all their problems with the patch have been
addressed.
My take on this is:
-2 - The approach taken to solve the problem is wrong and the whole thing should either be
abandoned or rewritten in a new way. I can only accept this though if the reviewer also
suggests the alternative (i.e. just saying your code isn't good is bad form imo).
e.g. stating things like 'circular references is bad' and giving -2 but not
suggesting alternatives and explanations is bad form imo.
-1 - I think there are some issues with the current patch that should be addressed *prior*
to merging it (bugs in the code that would affect many people etc). This would also
include complex code which needs explaining (if it's too complex for me to immediately
understand then it's fine to delay merge until either a good answer why this is
mandatory is received and what the code does or simplification of the code submitted or at
worst case - comment in the code.
-1 should only be given with proper explanation, otherwise imo it's bad form.
0 - I have some *personal* style problems, questions which do not affect the validity of
the patch *or* I think there are some changes that should be made but can definitely be
done in a future patch and should not prevent merging the current version. Note that I
find this very important to actually improve our current way of working. This means that
if a patch improves current code but could in itself be further improved, it is valid imo
to accept current version and ask committer to submit another patch to further improve
it.
Note that this would include things like (e.g.) discussions about spacing which are not
enforced by pep8 tools (i.e. preventing a patch which fixes bugs from going in because of
personal interpretation of pep8 about alignment of parameters in function signature is
wrong imo).
+1 - I have reviewed the code and it looks correct to me but I'm not a subject matter
expert / the maintainer.
Note that for things like style review only '+1' should be accompanied by a
cover commit message stating - "+1 only for style" as Doron has mentioned
previously on this thred.
+2 - I am a subject matter expert, I have reviewed the code and it looks good to me
(solves the problem properly and no serious issues left with it).
As Doron mentioned, in our group (storage) the standard is to have at least 2 reviews (by
different people) before committing unless the patch is *really* trivial.
This means that I try to avoid giving +2 if no else has given a +1 before.
Alon, note that we apply this both to vdsm and engine.
_______________________________________________
vdsm-devel mailing list
vdsm-devel(a)lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
7:51 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
On Tue, Oct 02, 2012 at 07:39:32PM -0400, Ayal Baron wrote:
Ayal, thanks for your thorough treatment of this subject :) I completely agree
with the framework that you have laid out here. Hopefully, we can all come to
an agreement on a quasi-official project-wide policy based on this and then
place it up on the wiki in a gerrit workflow best practices document.
IBM Linux Technology Center
First of all, in gerrit there is no immediately visible difference between '0'
and no review at all so someone might have serious issues with a patch but if she did not
mark it with -1 submitter might totally miss this fact.
esp. if someone sent a new revision and the title of the cover comment for previous
version doesn't state a -1 (so maintainer doesn't know he needs to go looking back
to verify things were fixed).
This is adding overhead on maintainer now to go back to each and every review and make
sure that there are no comments that should have been addressed in 0.
Note that if someone gave a -1, normally I'd expect that person to make sure and +1 a
subsequent patch to flag to maintainer that all their problems with the patch have been
addressed.
My take on this is:
-2 - The approach taken to solve the problem is wrong and the whole thing should either
be abandoned or rewritten in a new way. I can only accept this though if the reviewer
also suggests the alternative (i.e. just saying your code isn't good is bad form imo).
e.g. stating things like 'circular references is bad' and giving -2 but not
suggesting alternatives and explanations is bad form imo.
-1 - I think there are some issues with the current patch that should be addressed
*prior* to merging it (bugs in the code that would affect many people etc). This would
also include complex code which needs explaining (if it's too complex for me to
immediately understand then it's fine to delay merge until either a good answer why
this is mandatory is received and what the code does or simplification of the code
submitted or at worst case - comment in the code.
-1 should only be given with proper explanation, otherwise imo it's bad form.
0 - I have some *personal* style problems, questions which do not affect the validity of
the patch *or* I think there are some changes that should be made but can definitely be
done in a future patch and should not prevent merging the current version. Note that I
find this very important to actually improve our current way of working. This means that
if a patch improves current code but could in itself be further improved, it is valid imo
to accept current version and ask committer to submit another patch to further improve
it.
Note that this would include things like (e.g.) discussions about spacing which are not
enforced by pep8 tools (i.e. preventing a patch which fixes bugs from going in because of
personal interpretation of pep8 about alignment of parameters in function signature is
wrong imo).
+1 - I have reviewed the code and it looks correct to me but I'm not a subject matter
expert / the maintainer.
Note that for things like style review only '+1' should be accompanied by a
cover commit message stating - "+1 only for style" as Doron has mentioned
previously on this thred.
+2 - I am a subject matter expert, I have reviewed the code and it looks good to me
(solves the problem properly and no serious issues left with it).
As Doron mentioned, in our group (storage) the standard is to have at least 2 reviews (by
different people) before committing unless the patch is *really* trivial.
This means that I try to avoid giving +2 if no else has given a +1 before.
Alon, note that we apply this both to vdsm and engine.
--
Adam Litke <agl(a)us.ibm.com
10:39 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
* Ayal Baron <abaron(a)redhat.com> [2012-10-02 18:41]:
Ayal,
Thanks for taking time to write up your thoughts.
> >> A polite and well-detailed -1 should be used daily and
not
> >> considered
> >> "rude".
> >>
> >> Dan.
> >>
> >
> > Yes, this is the bookmark approach...
> >
> > The problem is that most people has the -1, 0, +1 scale, and in
> > that scale -1 is actually the reject method.
> >
> > "If I was the maintainer I would have rejected this patch".
>
> we could open -2 to non-maintainers as well if it is of use.
> danken?
Personally I disagree with this approach, -2 should rarely be given
and doesn't disappear. It should only be given to people who really
understand the code properly and whose opinion is well accepted by the
community.
I agree with Alon that 0 can and should be used, I do not agree with
him though that ack/nack type of discussion should take place on it
though and I'll explain why -
First of all, in gerrit there is no immediately visible difference
between '0' and no review at all so someone might have serious issues
with a patch but if she did not mark it with -1 submitter might
totally miss this fact. esp. if someone sent a new revision and the
title of the cover comment for previous version doesn't state a -1 (so
maintainer doesn't know he needs to go looking back to verify
things were fixed).
Absolutely. I've been worried about this specific issue. For me, at
least, the gui tends to hide quite a bit of the conversation in a normal
discussion of a patch. The comments section in the gui many times only
says '1 inline comment' and provides no direct link to the text of the
comment. Following the thread via email is much easier since all of the
comments are there and ordered, threaded. However, we currently can't
reply/participate via email and also include the comments back into the
gui for the other users.
So I've been concerned that conversations might be lost as new revisions
of the patches come in.
This is adding overhead on maintainer now to go back to each and
every
review and make sure that there are no comments that should have been
addressed in 0. Note that if someone gave a -1, normally I'd expect
that person to make sure and +1 a subsequent patch to flag to
maintainer that all their problems with the patch have been addressed.
In addition to maintainers, any community member attempting to help
with the review load, this is an issue as well.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
4:56 p.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
----- Original Message -----
* Ayal Baron <abaron(a)redhat.com> [2012-10-02 18:41]:
>
>
Ayal,
Thanks for taking time to write up your thoughts.
> > >> A polite and well-detailed -1 should be used daily and not
> > >> considered
> > >> "rude".
> > >>
> > >> Dan.
> > >>
> > >
> > > Yes, this is the bookmark approach...
> > >
> > > The problem is that most people has the -1, 0, +1 scale, and in
> > > that scale -1 is actually the reject method.
> > >
> > > "If I was the maintainer I would have rejected this patch".
> >
> > we could open -2 to non-maintainers as well if it is of use.
> > danken?
>
> Personally I disagree with this approach, -2 should rarely be given
> and doesn't disappear. It should only be given to people who
> really
> understand the code properly and whose opinion is well accepted by
> the
> community.
>
> I agree with Alon that 0 can and should be used, I do not agree
> with
> him though that ack/nack type of discussion should take place on it
> though and I'll explain why -
>
> First of all, in gerrit there is no immediately visible difference
> between '0' and no review at all so someone might have serious
> issues
> with a patch but if she did not mark it with -1 submitter might
> totally miss this fact. esp. if someone sent a new revision and
> the
> title of the cover comment for previous version doesn't state a -1
> (so
> maintainer doesn't know he needs to go looking back to verify
> things were fixed).
Absolutely. I've been worried about this specific issue. For me, at
least, the gui tends to hide quite a bit of the conversation in a
normal
discussion of a patch. The comments section in the gui many times
only
says '1 inline comment' and provides no direct link to the text of
the
comment. Following the thread via email is much easier since all of
the
comments are there and ordered, threaded. However, we currently
can't
reply/participate via email and also include the comments back into
the
gui for the other users.
So I've been concerned that conversations might be lost as new
revisions
of the patches come in.
I totally agree which is why I mark -1 if I have comments that I want submitter to reply
to either with 'Done' or with reason why I'm wrong or 'I'll submit
another patch to fix that' (in which case I may change a -1 to +1) etc.
> This is adding overhead on maintainer now to go back to each and
> every
> review and make sure that there are no comments that should have
> been
> addressed in 0. Note that if someone gave a -1, normally I'd
> expect
> that person to make sure and +1 a subsequent patch to flag to
> maintainer that all their problems with the patch have been
> addressed.
In addition to maintainers, any community member attempting to help
with the review load, this is an issue as well.
Indeed.
--
Ryan Harper
Software Engineer; Linux Technology Center
IBM Corp., Austin, Tx
ryanh(a)us.ibm.com
1:11 a.m.
New subject: Change in vdsm[master]: Use 'yum clean expire-cache' instead of 'yum clean all'
On 10/02/2012 10:02 AM, Dan Kenigsberg wrote:
On Tue, Oct 02, 2012 at 09:34:05AM -0400, Alon Bar-Lev wrote:
>
> ----- Original Message -----
>> From: "Dan Kenigsberg"<danken(a)redhat.com>
>> To: "Alon Bar-Lev"<alonbl(a)redhat.com>
>> Cc: "Doron Fediuck"<dfediuck(a)redhat.com>, "Mark
Wu"<wudxw(a)linux.vnet.ibm.com>, "Greg Padgett"
>> <gpadgett(a)redhat.com>, vdsm-devel(a)lists.fedorahosted.org, "Ryan
Harper"<ryanh(a)us.ibm.com>, "Ayal Baron"
>> <abaron(a)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"<dfediuck(a)redhat.com>
>>>> To: "Alon Bar-Lev"<alonbl(a)redhat.com>
>>>> Cc: "Mark Wu"<wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
>>>> <danken(a)redhat.com>, "Greg
Padgett"<gpadgett(a)redhat.com>,
>>>> vdsm-devel(a)lists.fedorahosted.org, "Ryan Harper"
>>>> <ryanh(a)us.ibm.com>, "Ayal
Baron"<abaron(a)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"<alonbl(a)redhat.com>
>>>>> To: "Ryan Harper"<ryanh(a)us.ibm.com>
>>>>> Cc: "Mark Wu"<wudxw(a)linux.vnet.ibm.com>, "Dan
Kenigsberg"
>>>>> <danken(a)redhat.com>, "Greg
Padgett"<gpadgett(a)redhat.com>,
>>>>> "Doron Fediuck"<dfediuck(a)redhat.com>,
>>>>> vdsm-devel(a)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"<ryanh(a)us.ibm.com>
>>>>>> To: vdsm-devel(a)lists.fedorahosted.org
>>>>>> Cc: "Mark Wu"<wudxw(a)linux.vnet.ibm.com>,
"Dan Kenigsberg"
>>>>>> <danken(a)redhat.com>, "Greg
Padgett"<gpadgett(a)redhat.com>,
>>>>>> "Doron Fediuck"<dfediuck(a)redhat.com>, "Alon
Bar-Lev"
>>>>>> <alonbl(a)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<alonbl(a)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.
--
Cheers
Douglas
4243
days inactive
4246
days old
vdsm-devel@lists.fedorahosted.org
18 comments
8 participants
participants (8)
-
abaron＠redhat.com -
agl＠us.ibm.com -
Alon Bar-Lev -
Dan Kenigsberg -
dfediuck＠redhat.com -
Douglas Schilling Landgraf -
iheim＠redhat.com -
Ryan Harper