Issue 3267: values of review carried from older than previous reviews
Status:  New
Owner: ----
Cc:
Reported by cascaran...@gmail.com, Mar 31, 2015
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
+1 me too. this is a very annoying issue.
Apr 8, 2015
#2 j...@flaska.net
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
#3 cascaran...@gmail.com
[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
#4 j...@flaska.net
If you do not want your Verified scores to be copied at all, specify copyAllScoresOnTrivialRebase and copyAllScoresIfNoCodeChange = false, then.
Apr 8, 2015
#5 cascaran...@gmail.com
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
#6 cascaran...@gmail.com
Please note that all the reviews are done with the same user.
Apr 8, 2015
#7 j...@flaska.net
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
#8 cascaran...@gmail.com
Exactly! Sorry if I misled you.
Apr 22, 2015
Project Member #9 edwin.ke...@gmail.com
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
#10 cascaran...@gmail.com
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
#11 cascaran...@gmail.com
Any updates here? It's not blocking but it's annoying