CandyShop / gerrit

Automatically exported from code.google.com/p/gerrit
Apache License 2.0
1 stars 0 forks source link

values of review carried from older than previous reviews #3267

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
Affected Version:
2.10

What steps will reproduce the problem?
1.Add a -CR (Code review) -V (verified) review to a change
2.Do a trivial rebase
3.Unset the -V value (set to 0) and add +CR
4.Do a trivial rebase

What is the expected output? What do you see instead?
The expected output is that the new change has the value +CR carried over, and 
no V flag value.
Instead it gets the +DR and the -V from two reviews before that, setting the 
final review as +CR -V.

Please provide any additional information below.

Original issue reported on code.google.com by cascaran...@gmail.com on 31 Mar 2015 at 3:41

GoogleCodeExporter commented 9 years ago
+1 me too. this is a very annoying issue.

Original comment by kovg...@gmail.com on 8 Apr 2015 at 11:45

GoogleCodeExporter commented 9 years ago
Could you please post a complete configuration of your labels from 
refs/meta/config? In particular, what is the range of these labels on your 
system, and what values of label.Foo.copy* options do you use for CR and V? 
This is explained at 
https://gerrit-review.googlesource.com/Documentation/config-labels.html .

Original comment by j...@flaska.net on 8 Apr 2015 at 1:49

GoogleCodeExporter commented 9 years ago
[label "Verified"]
    function = MaxWithBlock
    value = -1 Fails
    value =  0 No score
    value = +1 Verified
    copyAllScoresOnTrivialRebase = true
    copyAllScoresIfNoCodeChange = true
    defaultValue = 0
[label "Code-Review"]
    function = MaxWithBlock
    abbreviation = R
    copyMinScore = true
    value = -2 Do not submit
    value = -1 I would prefer that you didn't submit this
    value =  0 No score
    value = +1 Looks good to me, but someone else must approve
    value = +2 Looks good to me, approved
    copyAllScoresOnTrivialRebase = true
    copyAllScoresIfNoCodeChange = true
    defaultValue = 0

Original comment by cascaran...@gmail.com on 8 Apr 2015 at 2:01

GoogleCodeExporter commented 9 years ago
If you do not want your Verified scores to be copied at all, specify 
copyAllScoresOnTrivialRebase and copyAllScoresIfNoCodeChange = false, then.

Original comment by j...@flaska.net on 8 Apr 2015 at 2:14

GoogleCodeExporter commented 9 years ago
I do want them copied. But I don't want them to be ignore reviews that reset 
(put on 0) the flags.

Original comment by cascaran...@gmail.com on 8 Apr 2015 at 2:35

GoogleCodeExporter commented 9 years ago
Please note that all the reviews are done with the same user.

Original comment by cascaran...@gmail.com on 8 Apr 2015 at 2:37

GoogleCodeExporter commented 9 years ago
After five re-reads, I now understand that you're complaining that a trivial 
rebase in step #4 inherits the Verified-1 from #2 rather than copying the value 
explicitly assigned in #3.

Original comment by j...@flaska.net on 8 Apr 2015 at 4:51

GoogleCodeExporter commented 9 years ago
Exactly! Sorry if I misled you.

Original comment by cascaran...@gmail.com on 8 Apr 2015 at 5:02

GoogleCodeExporter commented 9 years ago
I believe this new behaviour is intended. Have a look at this change:
  https://gerrit-review.googlesource.com/53602

The commit message says:
"Second, we no longer pass in a "source" patch set; all prior patch
sets are considered. This has the advantage that it does not rely on
labels having been properly copied to intermediate patch sets. It also
means, for example, if patch set 3 has a code change relative to patch
set 2 but not patch set 1, approvals on a copyAllScoresOnNoCodeChange
label will be copied from patch set 1, bypassing patch set 2. We think
this is a more correct state of affairs: if patch set 1 is good enough
to submit, so should patch set 3, regardless of the fact that there is
an intermediate patch set."

Original comment by edwin.ke...@gmail.com on 22 Apr 2015 at 12:05

GoogleCodeExporter commented 9 years ago
the issue is happening when the patch 3 has code changes on 1 and 2, and it's 
not ignoring patch 2 reviews but mixing them (CR from 2 and V from 1)

Original comment by cascaran...@gmail.com on 22 Apr 2015 at 3:47

GoogleCodeExporter commented 9 years ago
Any updates here? It's not blocking but it's annoying

Original comment by cascaran...@gmail.com on 12 May 2015 at 9:51