Last modified: 2013-04-12 16:30:13 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 35883 - Unintuitive nagging for gerrit after review
Unintuitive nagging for gerrit after review
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
All All
: Low normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: testme, upstream
Depends on: 39589
  Show dependency treegraph
Reported: 2012-04-11 11:23 UTC by Marcin Cieślak
Modified: 2013-04-12 16:30 UTC (History)
4 users (show)

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


Description Marcin Cieślak 2012-04-11 11:23:51 UTC
Hello, I have casually. reviewed some change (just comments and the cover message, not score) and now this change is displayed BOLD on my list. Why? I want to have it on my list but I don't want it to be nagging me for something I have done.
Comment 1 Chad H. 2012-04-11 13:26:40 UTC
It's showing up as bold because the commit is wanting your review. This happens when one of two things happens:
1) Someone adds you manually to the review list, or
2) You review a patch, and then later a new patchset is submitted.

If the boldness or showing up in your view bugs you, you should be able to remove yourself from the review list.

(Also, if you're wanting to follow changes but not necessarily comment/review them, you can also star them)

I'm not really sure what needs fixing here?
Comment 2 Marcin Cieślak 2012-04-11 14:36:18 UTC
I would expect the behaviour you have described, too.

That's not the case. I did review the patch and there is no new patchset. This is Gerrit change #4658 - submitted Apr 10, 2012 21:59, my comment is Apr 11, 2012 and it is "bold" in my list. If there is nothing new after my comment, I shouldn't be bothered.
Comment 3 Chad H. 2012-04-11 14:45:48 UTC
Ah, that's because you left a comment but didn't review, so you were added to the review list. Maybe not intuitive, but that's life--we can file upstream for an enhancement "ability to leave comment without reviewing" if it doesn't already exist.

(The workaround listed above should still work)
Comment 4 Marcin Cieślak 2012-04-13 19:20:30 UTC
I've had a look at the code and I find it insane. It's amazing combination of very clever programming (hey, I recognized 80% of patterns used) with dirty hacks. Besides, the DB structure is completely denormalized. 

What happens is whenever somebody is invited to do a change ("Add reviewer") Gerrit adds an empty comment of type "Code Review" with value ... 0 (no -1 or +1). 

So it is exactly the same situation as adding an in-line or off-line comment with selecting "Code Review" - 0 ("No score"). That explains what comment 3 says: "so you were added to the review list".

So as long as it does not find a "Code Review" or "Verified" or whatever comment with -2, -1, +1 or +2 it will nag you anyway. 

By the way, Submit is a special case of review internally in case you didn't know (also hacked-in nastily imho). 

It actually never looks at the history within the patchset. 

I still wonder how to properly describe a desired workflow for the upstream bug.

It motivates me really to have a serious look at Phabricator...
Comment 5 Antoine "hashar" Musso (WMF) 2012-08-28 09:33:05 UTC
I guess the workaround is to +1/-1 the patch and it will no more be bold, hence I am closing this bug.
Comment 6 Marcin Cieślak 2012-08-28 12:47:46 UTC
Sorry, this *is* a bug :)

And that one even got fixed:

So marking this as waiting for 2.5
Comment 7 Chad H. 2013-02-19 17:17:00 UTC
This looks fixed to me...can you confirm?
Comment 8 Chad H. 2013-03-06 21:07:51 UTC
This probably will be even nicer once is merged.
Comment 9 Chad H. 2013-04-12 16:30:13 UTC
I'm marking this FIXED since the original bug (see comment 6) has been fixed & deployed. The junk in the previous comment ended up not working out.

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