Page MenuHomePhabricator

Gerrit: Preferences: change "reviewed" preference when visiting changes to unchecked
Closed, ResolvedPublic

Description

When I review code, the checkbox is set to "reviewed" by default. This is dangerous (you view and leave the code diff, and the code gets "ok" reviewed). It should be unchecked by default .

If possible, the standard setting ("reviewed=ok" or "reviewed=not ok" by default) could be perhaps become a user preferences option in the "Gerrit reviewer preferences", if you understand, what I mean.


Version: unspecified
Severity: normal

Details

Reference
bz35592

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:13 AM
bzimport added projects: Gerrit, Upstream.
bzimport set Reference to bz35592.
bzimport added a subscriber: Unknown Object (MLST).

That checkbox has to do with your own personal review of a file, not the actual review score given (-1/0/+1). There is zero danger here.

In any case, not urgent, should be reported upstream. Removing blocker.

But it appears as a green check on the main review page!

It makes sense to set the default to "unchecked"

(In reply to comment #2)

But it appears as a green check on the main review page!

Those are per-user...nobody sees those little checkmarks but you. They're designed for you to be able to track "I've reviewed this file" for your own purposes.

(In reply to comment #3)

It makes sense to set the default to "unchecked"

We can report it upstream.

(In reply to comment #4)

(In reply to comment #2)

But it appears as a green check on the main review page!

Those are per-user...nobody sees those little checkmarks but you. They're
designed for you to be able to track "I've reviewed this file" for your own
purposes.

I understand, but the green check appears (also) just when returning from a quick view to a changeset, even when I did not any code review.

Ok, so report the issue upstream...there's not much I can do here.

(In reply to comment #6)

Ok, so report the issue upstream...there's not much I can do here.

What exactly do you mean by "report the issue upstream" ?

(In reply to comment #9)

(In reply to comment #8)

http://code.google.com/p/gerrit/issues/entry

filed as https://code.google.com/p/gerrit/issues/detail?id=1314

back-ported their (gerrit team) answer to here:

"I think Martin already implemented this [1].

https://gerrit-review.googlesource.com/#/c/31130/ "

Are we at mediawiki running an older version ?

(In reply to comment #10)

Are we at mediawiki running an older version ?

We're running the 2.2.x version, which is the stable release. 2.3 is currently in release candidate status, and it looks like that will be released sometime next month.

So when bug 35466 is fixed, this will be too. Adding the dependency.

This didn't make it into 2.3, but it *is* in 2.4. So once that goes final and we upgrade, this will be fixed.

Marking LATER until we've got 2.4 on the timeline.

Filed the 2.4.1 upgrade bug, so adding dependency.

This is now fixed. In your patch preferences (viewable by Differences -> Preferences when viewing any patch) you can now check "manual review."