Page MenuHomePhabricator

Unintuitive nagging for gerrit after review
Closed, ResolvedPublic

Description

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.


Version: unspecified
Severity: normal

Details

Reference
bz35883

Related Objects

StatusSubtypeAssignedTask
ResolvedNone
Resolveddemon
Resolvedhashar

Event Timeline

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

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?

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.

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)

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...

I guess the workaround is to +1/-1 the patch and it will no more be bold, hence I am closing this bug.

This looks fixed to me...can you confirm?

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.