Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
garymm opened this issue Jan 24, 2019 · 10 comments

Comments

@garymm
Copy link

garymm commented Jan 24, 2019

My use case is:

  1. Someone leaves a bunch of comments.
  2. 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
Copy link
Owner

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?

@sdefresne sdefresne changed the title Make Work in Progress outgoing CL's not count as requiring my attention Make Work in Progress outgoing CL's (with comments) not count as requiring my attention Jan 29, 2019
@garymm
Copy link
Author

garymm commented Jan 29, 2019

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

@sdefresne
Copy link
Owner

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
Copy link
Author

garymm commented Jan 30, 2019

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
Copy link

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
Copy link
Author

garymm commented May 22, 2019

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

@zmbush
Copy link

zmbush commented Sep 25, 2019

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
Copy link
Contributor

adg commented Apr 16, 2020

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
Copy link
Owner

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants