gerrit-monitor
gerrit-monitor copied to clipboard
FR: Quieter notifications where an assignee is set
This is a great extension, but has some annoying qualities. A case I see a lot of is:
- CL has several people, including me, assigned as Reviewers
- one of these, not me, is also assigned as Assignee
- It shows up as a red "Incoming CL requiring your attention" notification for me anyway
I see a few potential solutions. I've ordered them by estimated work to implement, simplest first:
- If a CL has an Assignee set, Reviewers other than the Assignee see it lumped with stale CLs or with outgoing CLs requiring attention. Technically incorrect but an improvement on the status quo.
- If a CL has an Assignee set, Reviewers other than the Assignee see it as a new category, ranked just below "outgoing CLs requiring attention" in the icon-color-priority hierarchy.
- If a CL has ever had an Assignee set, Reviewers are ignored entirely unless they have unresolved comments on the CL.
- If a CL has ever had an Assignee set, Reviewers are ignored unless they have unresolved comments on the CL or the CL is stale.
I am not wedded to any of these suggestions, they're just ideas which are better than a blank page. I believe they are also in ascending order of how helpful they'd be, but the effort/helpfulness curve is not linear.
Gerrit has an option to mark a CL has Reviewed, and the extension (from version 1.0.11) will now respect it. Would this work for you (I don't think the flag is cleared until a new patch is uploaded)?
That's an improvement but I'd say it's ~20% as good as the requested feature. It is SOP in some of my organizations to add 3-10 people as reviewers effectively as FYIs, with only the assignee being expected to respond. Going to the CL and marking as reviewed is lower friction than removing yourself from the Reviewers list, but it's not as good as not getting a displayed notification.
On Tue, Jun 2, 2020, 06:07 Sylvain Defresne [email protected] wrote:
Gerrit has an option to mark a CL has Reviewed, and the extension (from version 1.0.11) will now respect it. Would this work for you (I don't think the flag is cleared until a new patch is uploaded)?
— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/sdefresne/gerrit-monitor/issues/19#issuecomment-637529941, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJR7Z5JVRLHB62X3E2PYNDRUT2QVANCNFSM4MDPB7QA .
Sorry about that.
I have no experience with the Assignee field as it is unused by all the project I'm tracking with the extension. As such, I'm not really sure how I can test your broken workflow and improve it. I would gladly accept a patch that would improve the situation when the Assignee field is used, as long as it does not break anything for project that do not use the field.
https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#get-assignee and https://gerrit-review.googlesource.com/Documentation/rest-api-changes.html#get-past-assignees are the relevant REST endpoints.
https://chromium-review.googlesource.com/q/project:chromiumos%252Fplatform%252Fec is a project which uses this workflow frequently
https://chromium-review.googlesource.com/q/is:assigned has all Chromium/ChromiumOS CLs which currently have an assignee.