Last modified: 2013-04-12 16:30:13 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 T37883, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 35883 - Unintuitive nagging for gerrit after review
Unintuitive nagging for gerrit after review
Status: RESOLVED FIXED
Product: Wikimedia
Classification: Unclassified
Git/Gerrit (Other open bugs)
unspecified
All All
: Low normal (vote)
: ---
Assigned To: Nobody - You can work on this!
: testme, upstream
Depends on: 39589
Blocks:
  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: ---


Attachments

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 :) 

http://code.google.com/p/gerrit/issues/detail?id=1407

And that one even got fixed:

https://gerrit-review.googlesource.com/#/c/35880/

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 https://gerrit-review.googlesource.com/#/c/42611/ 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.


Navigation
Links