Re: [vdsm] [Engine-devel] Copy reviewer scores on trivial rebase/commit msg changes

2014-01-21 Thread Adam Litke

On 18/01/14 01:48 +0200, Itamar Heim wrote:

I'd like to enable these - comments welcome:

1. label.Label-Name.copyAllScoresOnTrivialRebase

If true, all scores for the label are copied forward when a new patch 
set is uploaded that is a trivial rebase. A new patch set is 
considered as trivial rebase if the commit message is the same as in 
the previous patch set and if it has the same code delta as the 
previous patch set. This is the case if the change was rebased onto a 
different parent. This can be used to enable sticky approvals, 
reducing turn-around for trivial rebases prior to submitting a change. 
Defaults to false.



2. label.Label-Name.copyAllScoresIfNoCodeChange

If true, all scores for the label are copied forward when a new patch 
set is uploaded that has the same parent commit as the previous patch 
set and the same code delta as the previous patch set. This means only 
the commit message is different. This can be used to enable sticky 
approvals on labels that only depend on the code, reducing turn-around 
if only the commit message is changed prior to submitting a change. 
Defaults to false.


I am a bit late to the party but +1 from me for trying both.  I guess
it will be quite rare that something bad happens here.  So unlikely,
that the time saved on all the previous patches will far offset the
lost time for fixing the corner cases.
___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel


Re: [vdsm] [Engine-devel] Copy reviewer scores on trivial rebase/commit msg changes

2014-01-19 Thread Itamar Heim

On 01/19/2014 02:48 AM, Dan Kenigsberg wrote:

On Sat, Jan 18, 2014 at 01:48:52AM +0200, Itamar Heim wrote:

I'd like to enable these - comments welcome:

1. label.Label-Name.copyAllScoresOnTrivialRebase

If true, all scores for the label are copied forward when a new
patch set is uploaded that is a trivial rebase. A new patch set is
considered as trivial rebase if the commit message is the same as in
the previous patch set and if it has the same code delta as the
previous patch set. This is the case if the change was rebased onto
a different parent. This can be used to enable sticky approvals,
reducing turn-around for trivial rebases prior to submitting a
change. Defaults to false.


2. label.Label-Name.copyAllScoresIfNoCodeChange

If true, all scores for the label are copied forward when a new
patch set is uploaded that has the same parent commit as the
previous patch set and the same code delta as the previous patch
set. This means only the commit message is different. This can be
used to enable sticky approvals on labels that only depend on the
code, reducing turn-around if only the commit message is changed
prior to submitting a change. Defaults to false.


https://gerrit-review.googlesource.com/Documentation/config-labels.html


I think that the time saved by these copying is worth the dangers.

But is there a way to tell a human ack from an ack auto-copied by these
options? It's not so fair to blame X for X approved this patch when he
only approved a very similar version thereof.



we'll find out when we enable it.


Assuming that a clean rebase can do no wrong is sometimes wrong
(a recent example is detailed by Nir's http://gerrit.ovirt.org/21649/ )


of course it can do wrong, but that's the exception usually.

___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel


Re: [vdsm] [Engine-devel] Copy reviewer scores on trivial rebase/commit msg changes

2014-01-19 Thread Itamar Heim

On 01/19/2014 11:49 PM, Yair Zaslavsky wrote:



- Original Message -

From: Itamar Heim ih...@redhat.com
To: Dan Kenigsberg dan...@redhat.com
Cc: engine-devel engine-de...@ovirt.org, vdsm-devel@lists.fedorahosted.org
Sent: Sunday, January 19, 2014 10:20:33 AM
Subject: Re: [Engine-devel] Copy reviewer scores on trivial rebase/commit msg 
changes

On 01/19/2014 02:48 AM, Dan Kenigsberg wrote:

On Sat, Jan 18, 2014 at 01:48:52AM +0200, Itamar Heim wrote:

I'd like to enable these - comments welcome:

1. label.Label-Name.copyAllScoresOnTrivialRebase

If true, all scores for the label are copied forward when a new
patch set is uploaded that is a trivial rebase. A new patch set is
considered as trivial rebase if the commit message is the same as in
the previous patch set and if it has the same code delta as the
previous patch set. This is the case if the change was rebased onto
a different parent. This can be used to enable sticky approvals,
reducing turn-around for trivial rebases prior to submitting a
change. Defaults to false.


2. label.Label-Name.copyAllScoresIfNoCodeChange

If true, all scores for the label are copied forward when a new
patch set is uploaded that has the same parent commit as the
previous patch set and the same code delta as the previous patch
set. This means only the commit message is different. This can be
used to enable sticky approvals on labels that only depend on the
code, reducing turn-around if only the commit message is changed
prior to submitting a change. Defaults to false.


https://gerrit-review.googlesource.com/Documentation/config-labels.html


I think that the time saved by these copying is worth the dangers.

But is there a way to tell a human ack from an ack auto-copied by these
options? It's not so fair to blame X for X approved this patch when he
only approved a very similar version thereof.


I think the ideas are good, regarding a way to mark if this is human ack or 
not - can the process of copying post a comment that copying occurred?


we'll see when enabled it. I plan to do that tuesday if no strong 
objections will arise.








we'll find out when we enable it.


Assuming that a clean rebase can do no wrong is sometimes wrong
(a recent example is detailed by Nir's http://gerrit.ovirt.org/21649/ )


of course it can do wrong, but that's the exception usually.

___
Engine-devel mailing list
engine-de...@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-devel



___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel


Re: [vdsm] [Engine-devel] Copy reviewer scores on trivial rebase/commit msg changes

2014-01-18 Thread Antoni Segura Puimedon


- Original Message -
 From: Itamar Heim ih...@redhat.com
 To: Antoni Segura Puimedon asegu...@redhat.com, Greg Sheremeta 
 gsher...@redhat.com
 Cc: engine-devel engine-de...@ovirt.org, vdsm-devel@lists.fedorahosted.org
 Sent: Saturday, January 18, 2014 2:49:45 PM
 Subject: Re: [Engine-devel] [vdsm] Copy reviewer scores on trivial 
 rebase/commit msg changes
 
 On 01/18/2014 01:39 PM, Antoni Segura Puimedon wrote:
 
 
  - Original Message -
  From: Greg Sheremeta gsher...@redhat.com
  To: Itamar Heim ih...@redhat.com
  Cc: engine-devel engine-de...@ovirt.org,
  vdsm-devel@lists.fedorahosted.org
  Sent: Saturday, January 18, 2014 1:35:57 AM
  Subject: Re: [Engine-devel] [vdsm] Copy reviewer scores on trivial
  rebase/commit msg changes
 
 
 
  - Original Message -
  From: Itamar Heim ih...@redhat.com
  To: engine-devel engine-de...@ovirt.org,
  vdsm-devel@lists.fedorahosted.org
  Sent: Friday, January 17, 2014 6:48:52 PM
  Subject: [vdsm] Copy reviewer scores on trivial rebase/commit msg changes
 
  I'd like to enable these - comments welcome:
 
  1. label.Label-Name.copyAllScoresOnTrivialRebase
 
  If true, all scores for the label are copied forward when a new patch
  set is uploaded that is a trivial rebase. A new patch set is considered
  as trivial rebase if the commit message is the same as in the previous
  patch set and if it has the same code delta as the previous patch set.
  This is the case if the change was rebased onto a different parent. This
  can be used to enable sticky approvals, reducing turn-around for trivial
  rebases prior to submitting a change. Defaults to false.
 
 
  2. label.Label-Name.copyAllScoresIfNoCodeChange
 
  If true, all scores for the label are copied forward when a new patch
  set is uploaded that has the same parent commit as the previous patch
  set and the same code delta as the previous patch set. This means only
  the commit message is different. This can be used to enable sticky
  approvals on labels that only depend on the code, reducing turn-around
  if only the commit message is changed prior to submitting a change.
  Defaults to false.
 
  Do the above apply to verify+1? Cause I'd handle that separately.
  Verification
  should be done even after trivial rebase.
 
 we can decide for which label.
 for example, we can decide just changing the commit message doesn't
 clear the verification flag, but changing the code does require
 re-verification.

That sounds great.
 
 
 
 
  https://gerrit-review.googlesource.com/Documentation/config-labels.html
  ___
  vdsm-devel mailing list
  vdsm-devel@lists.fedorahosted.org
  https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel
 
  +1 from me on both. I think they're great features.
  ___
  Engine-devel mailing list
  engine-de...@ovirt.org
  http://lists.ovirt.org/mailman/listinfo/engine-devel
 
 
 
___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel


Re: [vdsm] [Engine-devel] Copy reviewer scores on trivial rebase/commit msg changes

2014-01-18 Thread Dan Kenigsberg
On Sat, Jan 18, 2014 at 01:48:52AM +0200, Itamar Heim wrote:
 I'd like to enable these - comments welcome:
 
 1. label.Label-Name.copyAllScoresOnTrivialRebase
 
 If true, all scores for the label are copied forward when a new
 patch set is uploaded that is a trivial rebase. A new patch set is
 considered as trivial rebase if the commit message is the same as in
 the previous patch set and if it has the same code delta as the
 previous patch set. This is the case if the change was rebased onto
 a different parent. This can be used to enable sticky approvals,
 reducing turn-around for trivial rebases prior to submitting a
 change. Defaults to false.
 
 
 2. label.Label-Name.copyAllScoresIfNoCodeChange
 
 If true, all scores for the label are copied forward when a new
 patch set is uploaded that has the same parent commit as the
 previous patch set and the same code delta as the previous patch
 set. This means only the commit message is different. This can be
 used to enable sticky approvals on labels that only depend on the
 code, reducing turn-around if only the commit message is changed
 prior to submitting a change. Defaults to false.
 
 
 https://gerrit-review.googlesource.com/Documentation/config-labels.html

I think that the time saved by these copying is worth the dangers.

But is there a way to tell a human ack from an ack auto-copied by these
options? It's not so fair to blame X for X approved this patch when he
only approved a very similar version thereof.

Assuming that a clean rebase can do no wrong is sometimes wrong
(a recent example is detailed by Nir's http://gerrit.ovirt.org/21649/ )

Dan.
___
vdsm-devel mailing list
vdsm-devel@lists.fedorahosted.org
https://lists.fedorahosted.org/mailman/listinfo/vdsm-devel