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

ci: add assign-reviewers.yml #32

Closed

Conversation

sbanerjee-quic
Copy link
Contributor

Assign code reviewers based on directory paths.

Assign code reviewers based on directory paths.

Signed-off-by: sbanerjee-quic <[email protected]>
@craigez
Copy link
Contributor

craigez commented Oct 7, 2024

Can we do this with a CODEOWNERS file? https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

That would require less maintenance.

@mynameistechno
Copy link
Contributor

Can we do this with a CODEOWNERS file? https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

Yes I was going to research that this week to see if it would work

@sbanerjee-quic
Copy link
Contributor Author

Can we do this with a CODEOWNERS file? https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners

Yes I was going to research that this week to see if it would work

I tried CODEOWNERS as well, but CODEOWNERS requires all participants to have write access.
BTW, I do agree this is more difficult to maintain CODEOWNERS.

@mynameistechno
Copy link
Contributor

Ah you’re write right https://github.com/orgs/community/discussions/29982

@mynameistechno
Copy link
Contributor

What if we used codeowners file to manage file paths and reviewers and then have a github action to assign based on that. Ie. I found this action that parses codeowners and tells you when files matched

https://github.com/marketplace/actions/codeowners-action

@sbanerjee-quic
Copy link
Contributor Author

What if we used codeowners file to manage file paths and reviewers and then have a github action to assign based on that. Ie. I found this action that parses codeowners and tells you when files matched

https://github.com/marketplace/actions/codeowners-action

We can try to adopt codeowners-action. This would mean there has to be a post processing of the JSON object to assign the reviews? I am not sure, but I suspect users with no-write access listed in CODEOWNERS file may have an fatal error when run, need to check though.

@mynameistechno
Copy link
Contributor

@sbanerjee-quic I'm thinking it may be easiest to just give these users Write permissions so we can use CODEOWNERS. They're all QC engineers anyway, not external folks. You just need to provide guidance for them not to merge PRs, and if they need to change anything they should not push code directly but should open a PR.

Any concerns with this @sbanerjee-quic @ricardosalveti ? If not I can update the Triage team to have write permissions? We could create a separate team from Triage but not sure there is much benefit

@ricardosalveti
Copy link
Contributor

@sbanerjee-quic I'm thinking it may be easiest to just give these users Write permissions so we can use CODEOWNERS. They're all QC engineers anyway, not external folks. You just need to provide guidance for them not to merge PRs, and if they need to change anything they should not push code directly but should open a PR.

That seems fine by me, we can align the repo/maintainance policy with the team, but @sbanerjee-quic knows more about who should have access to it.

@sbanerjee-quic
Copy link
Contributor Author

I am not too certain about letting the triage group write access. It may lead to unintended mistakes.

Is there a way to limit the merge to select people?

@sbanerjee-quic
Copy link
Contributor Author

The CODEOWNERS based PR #44

@mynameistechno
Copy link
Contributor

I left a comment here #23 (comment) regarding this

@mynameistechno
Copy link
Contributor

Closing this in favor of #44 and the following approach:

I enabled branch protection, and granted Write access to qc triage team for meta-qcom-hwe and meta-qcom-distro (the PR for codeowners #44 no longer has linting errors).

Once we merge #44, I will to update the branch protection to require 2 reviewers, and require codeowners review.

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

Successfully merging this pull request may close these issues.

4 participants