-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
Issue/PR auto-assign is making us miss contributions #1698
Comments
We (@mx-psi, @ericmustin and @KSerrania) are having similar issues with PRs made against the Datadog exporter. |
Any solution? |
I think we need to review the auto-assign feature being used to select an individual from the pool of reviewers. Ideally it would pick one from the (first) collector-contrib-approvers team, and subsequent entries in the CODEOWNERS file ass approvers. However, I'm not sure that is possible. An alternative would be manual process, where whenever any PR that directly affects a vendor managed exporter, there should be approvable from them first. The fallback solution, would be to disable the auto-assign feature to ensure all participants in the CODEOWNERS file are notified. |
I don't think this would work for this repo, Github docs say
which would not be the case here. |
Interesting, so CODEOWNERS only works with users who have write permissions. With that in mind, I think a general process of getting approval from a vendor contributor should be required before merging, alongside an approval from the OTel contrib approvers of course. |
Signed-off-by: Bogdan Drutu <[email protected]>
Have you tested if it works with Github docs also says
|
@open-telemetry/collector-contrib-maintainer should we try adding read access to test if notifications work? |
Has there been any resolution on this issue? We have a similar problem on the |
This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping |
Thanks to the improvements by @open-telemetry/collector-contrib-triagers I think the only thing missing here is to allow adding the component labels to PRs as well as issues. This would allow approvers/maintainers to more easily ping code owners. I think this would amount to changing the triggering conditions here
|
I think the reason my PR falls short is bc it causes us to have to maintain a second CODEOWNERS file essentially to take advantage of existing actions. It would be good to try a solution that uses the existing CODEOWNERS file and is able to detect the directory changes in the PR on every push. |
Also I haven't found an existing workflow yet that can automatically assign/request review for CODEOWNERS using the CODEOWNERS file. In general this problem is harder than what we've done for issues bc of the permissions needed to interact with a PR. |
Describe the bug
We have seen on a number of occasions where the auto-assign feature combined with CODEOWNERS does not notify all parties of a new issue / PR, against a sub-directory we should be. Myself, @paulosman & @lizthegrey are all in the CODEOWNERS file alongside the @open-telemetry/collector-contrib-approvers, for the Honeycomb exporter but the auto-assign feature picks someone from the overall group.
This has lead to a number of PRs being submitted, approved & merged without our knowledge:
#291
#1162
And most recently:
#1689
(@bogdandrutu did catch this after the fact, but it shouldn't be on an individual to remember / fix retrospectively)
I did originally raise this as part this issue as there was some discussion around too many people getting notified.
The text was updated successfully, but these errors were encountered: