Page MenuHomePhabricator

"Only comments" switch went missing with Gerrit 3.3
Closed, ResolvedPublicBUG REPORT

Description

Upstream https://bugs.chromium.org/p/gerrit/issues/detail?id=12991


With T48148: Allow hiding of non-discussion comments in Gerrit we got a super-helpful "Only comments" switch in the Gerrit UI that would hide all auto-generated comments. It appears like this broke with T262241: Upgrade Gerrit to 3.3. The switch is gone.

As far as I can tell that switch is a native Gerrit feature. See https://gerrit.wikimedia.org/r/Documentation/rest-api-changes.html and search for autogenerated:. It relies on our CI jobs to mark all non-comments as such. I had a look at the Gerrit documentation but it doesn't look like it's even possible to turn this feature off. So what broke?

Event Timeline

That link is useful insofar as it contains a further link to: https://www.gerritcodereview.com/2020-05-06-change-log-experiment.html which in turn contains this explanation:

  • The “Only Comments” toggle is renamed to “Show all entries” and is set to off by default.
  • The rules for when log entries are shown have changed: All human messages are shown, but for autogenerated messages they are only shown on one patchset. If for example a CI system posts on patchsets 3, 4, 5, 6, 7, and a Linter posts on patchsets 3, 5, 6, then the CI messages are only shown on ps 7, and the Linter messages are only shown on ps 6

Though this doesn't seem to work perfectly? Or at least not retroactively, e.g. see https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Wikibase/+/705653

But maybe it works good enough for new changes, e.g. https://gerrit.wikimedia.org/r/c/mediawiki/extensions/WikibaseLexeme/+/709747

Thanks for filing it, that got mentioned yesterday over IRC and from the quick investigation it is related to Change Log Experiment.

The new Show all entries is off by default. Human messages are shown, only the last autogenerated comments of a user is shown.

The consensus upstream had is on https://bugs.chromium.org/p/gerrit/issues/detail?id=8164#c29 , specially:

The current plan is to only show comments and everything else *for the latest snapshot*. We think that this is the best of both worlds:

  • You don't get 20 "uploaded patch set xx" messages and 20 outdated messages from CI
  • You do see the latest "uploaded patch set" message and the latest CI results.

From the upstream commit, there is a screenshot showing all the human messages and the CI results that matter (the ones for the current patchset):

new.png (820×1 px, 199 KB)

As mentioned on the upstream feature request, when all autogenerated comments are hidden, one would often want to show them to access the latest CI result which in turns showed them all as well as all the uploaded patchset spam. With the new view, you get the CI report immediately accessible, at the expanse of having a few more messages shown.

I guess there is no ideal solution, but that one saves a few click since most of the time people actually want to have a look at the CI result?

Feedbacks are gathered upstream under Experiment=ChangeLog and feedback can be filed.

Hm. The (so far) only thing I slightly disagree with is that it appears to show all auto-generated Added to reviewer:, Removed from reviewer:, Moved from cc to reviewer: and such. I feel these are "spammy" as well and should not be shown by default.

Everything else makes a lot more sense with this explanation. I agree it's a good idea. Thanks!

hashar claimed this task.

Marking this one solved after my previous comment. That is a design choice by upstream. It might well change again in a later Gerrit version (we run 3.3, latest is 3.5).