Make Work in Progress outgoing CL's (with comments) not count as requiring my attention

sdefresne / gerrit-monitor

Source for Gerrit Monitor Chrome extension

Apache License 2.0

21 stars 16 forks source link

Make Work in Progress outgoing CL's (with comments) not count as requiring my attention #2

Open garymm opened 5 years ago

garymm commented 5 years ago

My use case is:

Someone leaves a bunch of comments.
I decide that I need to reconsider the approach or do some other things before continuing the review. I mark the change as "work in progress".

After this the CL doesn't really need my attention.

sdefresne commented 5 years ago

I think currently all WIP changes are considered as requiring attention because I sometimes upload my CL and forget to assign reviewers. So, I think it would be a regression to consider all WIP as not requiring attention.

However, I can probably change so that WIP change with at least one comment or an assigned reviewer are considered as not requiring attention. This should cover your case, without causing regression for recently uploaded CLs.

Would this work for you?

garymm commented 5 years ago

If you just upload a CL and forget to assign reviewers, the default status is not WIP, is it?

sdefresne commented 5 years ago

The default status of uploaded CL without assigned reviewer is WIP (at least this is the case for the Gerrit installation used by Chromium).

garymm commented 5 years ago

For my most commonly used Gerrit hosts, the default state is "active".

Since it seems to be a configuration option in Gerrit, could we make this a configuration option in the plugin so users can choose the behavior?

nfischer commented 5 years ago

If you just upload a CL and forget to assign reviewers, the default status is not WIP, is it?

The default (git push origin HEAD:refs/for/master) is not WIP, but some teams build wrapper scripts to upload CLs, and those wrapper scripts might pass the WIP option to the Gerrit backend. Ex. Chromium's git cl upload script uses the WIP option but Android's repo upload does not.

garymm commented 5 years ago

Given this plugin is not chromium-specific, could we make the behavior not chromium-specific?

zmbush commented 5 years ago

I would love a configuration option. I often have 10+ CLs marked as WIP that I don't intend to send out for review yet, and it would be great if I could make them not appear in the plugin.

adg commented 4 years ago

My pull request #21 should fix this. Or at least address it, by putting WIPs in a separate category to those that are simply without reviewer.

sdefresne commented 4 years ago

I've merged pull request #21 and published a new version on the Chrome store. This is going to be called version 1.0.8. Due to the current global situation, the review may take some time (up to 30 days), so I don't really know when the version is going to be available.

Once the new version become available, could you tell whether this is an improvement for you?

studgeek commented 2 years ago

If the gerrit server has attention set, then I would suggest it should just follow that. That would allow user to turn off notifications by removing themselves from the attention set in gerrit. However it seems that even with "Only use Attention Set to judge required attention" is enabled, Gerrit Monitor currently notifies for cases beyond attention set - for example approved #43 and new CLs #42.