Attention is currently required from: Arthur Heymans, Nico Huber, Jonathan Zhang, Johnny Lin, David Hendricks, Stefan Reinauer, Christian Walter, Deomid "rojer" Ryabkov, Tim Chu.
Patch set 3:Code-Review +2
1 comment:
Patchset:
The thing with -2's is that they prevent submitting changes, so people with -2 rights can veto other people's changes. This can easily be abused, which is why only a few people have -2 rights along with submit rights and the ability to give Verified scores (normally, the Jenkins CI gives Verified scores to patches after testing them).
In contrast, a -1 doesn't stop a change from being merged, but lets others know that someone doesn't agree with something about the change (ideally, the -1 would be accompanied by comments explaining why). Because of this, people (change authors in particular) usually react peacefully and constructively to a -1, learning from their mistakes and/or working towards a better approach.
Personally, I only -2 a change when it's completely and objectively wrong, and the reasoning I provide in a comment leaves nothing else to discuss. CB:49823 and CB:56065 are two examples of this: both changes were abandoned by the author after acknowledging my point.
When I actually want to discuss things, I usually go with just an unresolved comment. If a change has several +2's and is about to be merged, I may also give a -1 to raise more attention. The -1 doesn't unconditionally block submission, but delays it so that the matter can be discussed (submitting a change with unresolved comments and a -1 is an insultingly rude offense). Still, I hardly ever need to -1 changes in this situation.
I don't give negative review scores if I don't need to. Even a -1 can upset people or make them get defensive, which hinders cooperation. Commenting without giving any scores is usually a very good option.
TL;DR: Please do not give a -2 to changes when there's something to discuss. Only when a change is completely unsalvageable (i.e. fundamentally wrong and completely unfixable) is giving a -2 the right thing to do.
To view, visit change 57589. To unsubscribe, or for help writing mail filters, visit settings.