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

add CODEOWNERS file #18

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

add CODEOWNERS file #18

wants to merge 1 commit into from

Conversation

glimchb
Copy link
Contributor

@glimchb glimchb commented Feb 20, 2025

this helps to show who is reviewing code and mages notifications

this helps to show who is reviewing code and mages notifications
@@ -0,0 +1 @@
* @karlatec @tomzawadzki
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO Simon should be here as well

@karlatec
Copy link
Contributor

and mages notifications

Typo "manages"?

@tomzawadzki
Copy link
Contributor

I'm not sure what purpose does it serve to specify CODEOWNERS at this time. Currently only SPDK Core Maintainers have the write permission on the repository (only those can be in CODEOWNERS). Neither Karol or Simon do at this time.

Meanwhile this will be needed once more people are contributing, especially with workflows that they know best and/or own runners for them. Lets come back to this once the process for running the CI is much more clear and working.

@glimchb
Copy link
Contributor Author

glimchb commented Feb 21, 2025

I'm not sure what purpose does it serve to specify CODEOWNERS at this time. Currently only SPDK Core Maintainers have the write permission on the repository (only those can be in CODEOWNERS). Neither Karol or Simon do at this time.

Meanwhile this will be needed once more people are contributing, especially with workflows that they know best and/or own runners for them. Lets come back to this once the process for running the CI is much more clear and working.

  1. it adds visibility who is waiting on approvals
  2. it adds notifications when PR is open
  3. it allows to split owners per file
  4. we can also create github team for core maintainers and add a team as owner

@mhae
Copy link

mhae commented Feb 21, 2025

I would definitely want to be the ultimate decider to anything that's related to our self-hosted runner.
The other files are okay and we probably don't need to have codeowners. But for security reasons, I would not want anyone besides us to make changes to the yaml that uses our self-hosted runner. I

Copy link

@mhae mhae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

Successfully merging this pull request may close these issues.

4 participants