Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gerrit: eliminate negative scores #16764

Closed
josharian opened this issue Aug 17, 2016 · 6 comments
Closed

gerrit: eliminate negative scores #16764

josharian opened this issue Aug 17, 2016 · 6 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@josharian
Copy link
Contributor

I propose that we eliminate negative scores from gerrit and add a separate "Hold" or "Do not submit" flag that prevents submission.

Negative scores serve two purposes. First, to convey that the reviewer is not happy with the CL. Second, in the case of a -2, to block submission.

Negative opinions about a CL need to occur in English, for two reasons. First, humans don't like negative feedback, and written language offers the opportunity to give negative feedback gracefully and with shades of meaning. Second, the negative feedback must explain what is wrong, so that the contributor can improve the CL (if possible) and can learn for the future.

Given that negative opinions must be accompanied by text, the negative score adds nothing except possibly negative emotion.

As to blocking submission, this need can occur independently of the reviewer's opinion of a CL. For example, I would like to be able to +2 CLs sent during a freeze and also block them to avoid accidental submission. And using a -2 to block submission can still carry some of the emotional weight of a negative score. (Humans are irrational. That's life.)

I don't know about the technical feasibility/reasonability of adding a "Do not submit" flag. If it is infeasible, we could keep the -1 score and document that its sole purpose is to block submission, not to provide feedback, although this would be non-ideal.

We would also need to update all accompanying documentation around contribution, although I expect that this would minimal.

@josharian josharian added this to the Unreleased milestone Aug 17, 2016
@matloob
Copy link
Contributor

matloob commented Aug 17, 2016

I'm not sure how easy it is possible to implement this in gerrit, but it seems like great idea to me.

@bradfitz
Copy link
Contributor

I believe this is technically just a Gerrit config change.

@broady
Copy link
Contributor

broady commented Aug 17, 2016

I like Hold. Let's go with that.

@ALTree ALTree added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Sep 22, 2018
@josharian
Copy link
Contributor Author

Ping. If nothing else, a Hold flag would be useful.

@josharian
Copy link
Contributor Author

@rsc just did this. Yay! 🤞🏽that it works well in practice.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. labels Apr 29, 2022
gopherbot pushed a commit to golang/website that referenced this issue Apr 29, 2022
Code-Review -2 is replaced by Hold +1.
Also update for recent review changes requiring two Google employees.

For golang/go#16764.
For golang/go#40699.

Change-Id: I6bf575e39309fdb0e25b5f741c39c71f23011334
Reviewed-on: https://go-review.googlesource.com/c/website/+/403054
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@gopherbot
Copy link

Change https://go.dev/cl/403054 mentions this issue: _content/doc/contribute: document new Hold label

@golang golang locked and limited conversation to collaborators Apr 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

7 participants