-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Approvers are receiving GitHub notifications for every PR #1339
Comments
+1 on the idea. @bogdandrutu what was the problem that required all approvers to be listed for all directories? |
I didn't know about this until right now, but GitHub does have a feature that automatically assigns people as reviewers and has the ability to not notify the whole group: Doc about this feature: https://docs.github.com/en/free-pro-team@latest/github/setting-up-and-managing-organizations-and-teams/managing-code-review-assignment-for-your-team |
I like the idea of auto-assigning a single approver per PR to help reduce the noise generated to the whole group. However, we have seen two cases where a vendor span exporter has had PRs merged without approval from a vendor representative. There is a specific rule setup in the CODEOWNERS file for the honeycomb exporter, but as the process selects only one approver, we just didn't get notified. @paulosman @lizthegrey @open-telemetry/collector-contrib-maintainer |
In the meantime, the following query works for me to find out which reviews are waiting for me individually:
|
Ping @tigrannajaryan, @bogdandrutu |
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
@bogdandrutu Could we give a try to the Github feature that @jpkrohling commented about in #1339 (comment) ? The auto assign Github Action we use seems to have some issues (e.g. PR assignment is not balanced across approvers and it is sometimes a bit slow) and it could help fix this issue |
Folks, it's becoming really hard to keep the sanity while working with OpenTelemetry. Right now, I have 148 notifications in my queue for opentelemetry-collector and opentelemetry-collector-contrib, and it's been only one business day since I had my queue cleared. I doubt they are all relevant. In fact, I doubt even 20 of them are relevant to me. I'm really about to resign as approver, as triaging this is consuming too much of my time. |
I echo the concerns above, it is very time-consuming to go through the notifications even for some one who is an approver only for the contrib repo. It should be easy to at least try the "Code review assignment" feature so that we are able to manage this better. |
I'd appreciate if we could try out the code review assignment technique too, if it works can close #3337 without doing anything as it would probably solve my current notification overload issue. |
@jpkrohling sorry, just saw your proposal above about auto-assigment. So, instead of assigning all approvers to each PR which will round-robin PRs? |
It can use round-robin or load balancing so that folks with an already big review queue won't keep getting new items to review. |
@jpkrohling does that also stop the notifications? |
My understanding is that the notifications are sent because the group is assigned to review the PR. If the assignee is now an individual, the notification should stop as well. |
Changed contrib as well, we need to make a decision about the auto-assignment script, which runs in parallel and may assign a different person as |
I just looked at a recently opened PR (#3701) and noticed that it got assigned to the team, instead of to an individual. The interesting part is this: Clicking on "code owners" led me to this:
Apparently, we might need to remove the group from the codeowners. |
I see the assigned person as the one responsible for making sure the PR is moving (reviewed, up to date, merged). I wouldn't mind being assigned to issues in addition to get review assignments. |
The latest changes did have an effect, and I'm now getting manageable levels of notifications! Thank you! |
* Bump github.com/google/go-cmp from 0.5.2 to 0.5.3 Bumps [github.com/google/go-cmp](https://github.com/google/go-cmp) from 0.5.2 to 0.5.3. - [Release notes](https://github.com/google/go-cmp/releases) - [Commits](google/go-cmp@v0.5.2...v0.5.3) Signed-off-by: dependabot[bot] <support@github.com> * Auto-fix go.sum changes in dependent modules Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: dependabot[bot] <dependabot[bot]@users.noreply.github.com> Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
…#1339) * collect threads count in opentelemetry-instrumentation-system-metrics * update * avoid devidedByZero exception when sawp memory is 0 * add ut * change log * lint * lint Co-authored-by: Srikanth Chekuri <srikanth.chekuri92@gmail.com>
Even with the inclusion of the bot that auto assigns people as reviewers, everyone in the approvers group is receiving a notification to review all the issues. Example:
Notifications page:
PR page:
In order to not cause "alert fatigue" to approvers, the group should not be added as reviewer, but instead, a specific person chosen by the bot.
The text was updated successfully, but these errors were encountered: