PolyGerrit dashboard: Which changes need /my/ review?

[Ilya Sherman]

I've been having a consistent point of frustration with the PolyGerrit dashboard UI: I can't tell which "Incoming reviews" actually need my attention.  From what I can tell, a line is bolded if it matches "-reviewedby:self", i.e. I haven't reviewed the most recently uploaded patch set.  In Rietveld, and also in Google's internal code review tool, lines are colored green if I have already approved the CL -- I think that matches "label:Code-Review=+1,self".  Could we implement the same in the PolyGerrit UI?  (Should I just file a bug?  I'm asking here first because I'd guess this has already come up, and I'm just failing to find the prior discussion...)

Also, while I have your attention: I find the "CR" column of the dashboard somewhat confusing.  Does a checkmark in that column just mean that at least one person has +1'ed the CL?  I think that's what it means, and I don't really understand how that information is helpful to me, either as a CL author or as a reviewer.  As a reviewer, I want to know whether the CL needs my attention.  As an author, I want to know whether the CL has full OWNERship approval.  Neither of these aligns well with "at least one reviewer +1'ed the CL".  Am I holding it wrong?

Thanks!
~ilya

[Primiano Tucci]

Strongly echo this. I ended up missing some reviews because of not being able to tell whether I need to take action on a cl as a reviewer.
I filed https://bugs.chromium.org/p/gerrit/issues/detail?id=6659 a while ago but got no response.

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAA3nRagUah%3DtYnb%2BCQUUO6LkHqPnaSSiJZUPnsWSWhQK0cJoSA%40mail.gmail.com.

[Scott Graham]

I don't use the dashboard for either system, but I also have no idea what I'm supposed to be reviewing because the initial emails are content-free. This is how reviews start right now:

"""

Xyz posted comments on this change.

View Change

Patch set 1:

This change is ready for review.

To view, visit change 588387. To unsubscribe, visit settings.

"""

Rietveld sends a much more actionable summary, like this:

"""

Reviewers: scottmg

CL: https://codereview.chromium.org/1234567/

Message:

...

Description:

...

Affected files (+N, -M lines):

...

"""

The "Reviewers:" line, and inline "Description:" being the most important parts.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CA%2ByH71cAMbUyOmQ0T97FJLWXPtBujVR2O-Sekrfcw_38QsbbfQ%40mail.gmail.com.

[Aaron Gable]

Yes, please file bugs using the "Send feedback" link at the bottom of the page! Those bugs auto-cc the techleads of PolyGerrit, so they definitely get looked at.

Regarding the specific complaints in this thread:

* Email is being handled in a different thread and in different bugs, but I can point to at least once recent change which increases the readability of Gerrit's emails and which should be released to production soon.

* We haven't had specific discussion about the use of the checkmark columns; thank you for filing that bug, and I'm sorry that it didn't get triaged sooner (now that it is in the Polygerrit component it should get looked at). It may be difficult to change directly since there are thousands of non-Chromium developers who are already accustomed to what those columns mean, but the request for a more direct indication of the need for your involvement is a good one.

* Side note: Gerrit has the concept of "assignee" for reviews -- if a review is assigned to you, it is highlighted in your dashboard, and it is clear that the next action taken needs to be yours. Feel free to use that, or not, if you think it will be helpful.

Aaron

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CANHK6RaTkdWtqwXwuAVF21uMUoYefRcGit%2BiQRvtEpxJn%3DjkCw%40mail.gmail.com.

[Ilya Sherman]

Thanks, Aaron!

On Fri, Jul 28, 2017 at 5:48 AM, Aaron Gable <aga...@chromium.org> wrote:

Yes, please file bugs using the "Send feedback" link at the bottom of the page! Those bugs auto-cc the techleads of PolyGerrit, so they definitely get looked at.

Regarding the specific complaints in this thread:

* Email is being handled in a different thread and in different bugs, but I can point to at least once recent change which increases the readability of Gerrit's emails and which should be released to production soon.

* We haven't had specific discussion about the use of the checkmark columns; thank you for filing that bug, and I'm sorry that it didn't get triaged sooner (now that it is in the Polygerrit component it should get looked at). It may be difficult to change directly since there are thousands of non-Chromium developers who are already accustomed to what those columns mean, but the request for a more direct indication of the need for your involvement is a good one.

Could we perhaps address this by using a different dashboard template for Chromium and related projects?

[Aaron Gable]

We can't do something quite as simple as using a different template -- all of PolyGerrit is served as static Polymer components, and the backend has no way to serve a different component based on who is requesting it or what project they're viewing. Gerrit does have an extensive plugin API, but that API doesn't (yet) have any hooks for modifying the behavior of the dashboard -- modifying the display of the change review page has been considered a much higher priority. I think this is all stuff that will get discussed in more detail in that bug.

[Wez]

Could we at least provide a way to explicitly mark a CL as "done with" from the reviewers PoV?  I currently have three CLs that are stuck bold in my review queue because they've been replied to by other reviewers, or seemingly even replied to by commit-bot.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAH58R2fBPay4D8xwXxvp1qyirW67a3okuDT%3DqWk4pLJ5RGot%2BQ%40mail.gmail.com.

[Aaron Gable]

Yeah! That's a totally reasonable feature request to file via the "Send Feedback" link at the bottom of any PolyGerrit page. Note, however, that it is a feature request rather than a regression, and will be prioritized as such.

[Sadrul Chowdhury]

FWIW: chromite-butler shows some hints on the dashboard:

. Adds green background color for the CLs I have lgtm'ed (and red for the ones not-lgtm'ed).

. Adds the list of reviewers for each CL (reviewer-name color-coded as above).

screenshot here.

Sadrul

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CALekkJe1b%2BGXFRy6bqW94DpNcnhxV3pPLnS1%3DWgf6QKCLzj_zg%40mail.gmail.com.

[Reilly Grant]

Given that we are now actively maintaining our code review system I hope we can merge many of the enhancements provided by chromite-buttler into Gerrit itself.

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CALekkJe1b%2BGXFRy6bqW94DpNcnhxV3pPLnS1%3DWgf6QKCLzj_zg%40mail.gmail.com.

--

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAKgcGXjfAUd-%2Byf0vUzW81tNGtA%3Dbz95zc%3D%3DthPhAjWOJ4dS3Q%40mail.gmail.com.

[Aaron Gable]

Yes, as mentioned above the plan is to turn chrome-butler into a Gerrit Plugin rather than an extension. At that point it will be owned and maintained by the Chrome Ops (nee Chrome Infra) team, and will be automatically deployed to all users of chromium.googlesource.com, not just those who install an extension. 

[Michael Giuffrida]

chromite-butler is super useful for this. I also discovered some native Gerrit features:

Mute: The CL acts like you've reviewed it. It stays non-bold in your reviews list until a new patchset is uploaded

Ignore: Stop getting emails about a CL. You do get notified when you're newly added as a reviewer or assignee, so this is akin to Gmai's "mute".

Apart from having confusing names, these seem like useful tools. You can find them in the More dropdown of a CL view. See: https://gerrit-review.googlesource.com/Documentation/dev-stars.html#ignore-star