| Issue 3267: | values of review carried from older than previous reviews | |
| 3 people starred this issue and may be notified of changes. | Back to list |
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.
Apr 8, 2015
#1
kovg...@gmail.com
Apr 8, 2015
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 .
Apr 8, 2015
[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
Apr 8, 2015
If you do not want your Verified scores to be copied at all, specify copyAllScoresOnTrivialRebase and copyAllScoresIfNoCodeChange = false, then.
Apr 8, 2015
I do want them copied. But I don't want them to be ignore reviews that reset (put on 0) the flags.
Apr 8, 2015
Please note that all the reviews are done with the same user.
Apr 8, 2015
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.
Apr 8, 2015
Exactly! Sorry if I misled you.
Apr 22, 2015
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."
Cc:
dborowitz@google.com
Apr 22, 2015
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)
May 12, 2015
Any updates here? It's not blocking but it's annoying |
|
| ► Sign in to add a comment |