Issue metadata
Sign in to add a comment
|
"CodeReview +1" should be shown also when somebody already +1'd a CL. |
||||||||||||||||||
Issue descriptionAffected Version: What steps will reproduce the problem? 1. Find a CL where you are able to vote "+1" on CodeReview label 2. Ask someone else to vote "+1" on it 3. Open the CL in your browser What is the expected output? "CodeReview +1" should still be visible. What do you see instead? "CodeReview +1" is not visible. Please provide any additional information below. Frequently this is useful when the originator of the first +1 vote is asking the author to get a review from someone else. The second reviewer will have to use "Send" button to execute +1. In fact I think +1 should always be visible in the menu when it's possible to vote +1 in the "Send" popup.
,
Mar 29 2017
I think it's the "Quick approve" behavior in question. Current logic is that if only one label from the set of required is missing, that label generates a quick vote button. So, if there is Verified and Code Review, and a user has +1 and +2 for those, Code Review+2 button will only be displayed when someone set Verified+1.
,
Mar 29 2017
No, it's about having CodeReview +1 (LGTM) button even if someone else has already +1'd (LGTM'd) the CL.
,
Apr 4 2017
The ask here is to modify the logic of the quick-approve button. However, it's not so easy to see how to combine a persistent +1 button with the other "quick approve" states. For example, the quick-approve might normally be [Verified +1] or [Custom-Label +3], in which cases it should *not* become a [Code-review +1] button. I am also not sure whether we know in-general which label to use for Code-Review.
,
Apr 5 2017
I think I understand now. IIUC, "quick approve" state is a project-wide (or host-wide) configured label to be applied to CLs that would typically approve the CL to be merged or rebased into the repo. Seems like for Chromium this is "CodeReview +1". Can we always show the "quick approve" button regardless of the state of CL unless it's blocked by some other rules? For Chromium it would work well, since we always allow CodeReview to be set to +1.
,
Apr 7 2017
The quick-approve state logic is subtly different. The CL declares which labels it's "missing" (i.e. it doesn't have but are needed to be able to submit). The label in the quick-approve is the highest score that the current user can apply to a missing label. (I believe it's the last declared label if multiple labels apply.) As such, there isn't a project-wide config regarding which label to prefer in general. In other words, if there is already a +1, then it won't be "missing", so the quick approve logic wouldn't know what label to show. I wonder if it would make sense for Chromium to add a [CodeReview +1] action via a plugin (and perhaps even hide the quick-approve action too). Do other labels ever appear in the quick-approve button?
,
Apr 8 2017
I'd also CC tandrii@, but have no rights to add CC here.
,
Apr 10 2017
,
Apr 11 2017
> I wonder if it would make sense for Chromium to add a [CodeReview +1] action via a plugin (and perhaps even hide the quick-approve action too). Do other labels ever appear in the quick-approve button? I think Plugin for Chromium would be best, particularly since Chromium projects don't have any other required labels.
,
Apr 20 2018
,
Jan 10
This bug had an unsupported status. Updating to Untriaged so someone will reevaluate.
,
Jan 10
|
|||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||
Comment 1 by logan@google.com
, Mar 29 2017