Last modified: 2012-07-06 15:08:39 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T37592, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 35592 - Gerrit: Preferences: change "reviewed" preference when visiting changes to unchecked
Gerrit: Preferences: change "reviewed" preference when visiting changes to un...
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: upstream
Depends on: 37406
Blocks:
  Show dependency treegraph
 
Reported: 2012-03-29 17:19 UTC by T. Gries
Modified: 2012-07-06 15:08 UTC (History)
3 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments

Description T. Gries 2012-03-29 17:19:49 UTC
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.
Comment 1 Chad H. 2012-03-29 17:31:39 UTC
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.
Comment 2 T. Gries 2012-03-29 17:38:44 UTC
But it appears as a green check on the main review page!
Comment 3 T. Gries 2012-03-29 17:39:17 UTC
It makes sense to set the default to "unchecked"
Comment 4 Chad H. 2012-03-29 17:47:26 UTC
(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.
Comment 5 T. Gries 2012-03-29 17:55:26 UTC
(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.
Comment 6 Chad H. 2012-03-29 18:55:29 UTC
Ok, so report the issue upstream...there's not much I can do here.
Comment 7 T. Gries 2012-03-29 20:25:16 UTC
(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" ?
Comment 10 T. Gries 2012-03-30 10:46:37 UTC
(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 ?
Comment 11 Chad H. 2012-03-30 11:27:29 UTC
(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.
Comment 12 Chad H. 2012-05-04 18:21:36 UTC
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.
Comment 13 Chad H. 2012-06-08 12:44:04 UTC
Filed the 2.4.1 upgrade bug, so adding dependency.
Comment 14 Chad H. 2012-07-06 15:08:39 UTC
This is now fixed. In your patch preferences (viewable by Differences -> Preferences when viewing any patch) you can now check "manual review."

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links