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 #252

Merged
merged 2 commits into from
Feb 5, 2024
Merged

Add CODEOWNERS #252

merged 2 commits into from
Feb 5, 2024

Conversation

marcelamelara
Copy link
Contributor

Adding a CODEOWNERS file for future PR notifications etc.

Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Copy link
Contributor

@mlieberman85 mlieberman85 left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@SecurityCRob SecurityCRob left a comment

Choose a reason for hiding this comment

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

+1

@marcelamelara
Copy link
Contributor Author

The spell checker is unhappy about the word "codeowners". I can upload a file that makes the error go away, if folks think that's necessary.

@mlieberman85
Copy link
Contributor

I think we should turn off the spell check being a blocker.

@lehors
Copy link
Contributor

lehors commented Jan 24, 2024

I think we should turn off the spell check being a blocker.

Actually that was already done. We can ignore the errors and merge anyway.

Otherwise I must say that I'm torn about adding this file because it is yet one more piece of data to maintain but I understand the benefits so I'm not opposed to it.

@marcelamelara
Copy link
Contributor Author

marcelamelara commented Jan 24, 2024

Otherwise I must say that I'm torn about adding this file because it is yet one more piece of data to maintain

Actually, if there's a Github team for the TAC, it may reduce overhead to add the team as the codeowner, rather than individuals in here. This way, whenever the team is updated when the TAC changes members, we still don't need to make changes to this file.

Signed-off-by: Marcela Melara <marcela.melara@intel.com>
Copy link
Contributor

@lehors lehors left a comment

Choose a reason for hiding this comment

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

Thank you! This last commit addresses my concern.

Copy link
Member

@steiza steiza left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Once this lands, we can check "Require review from Code Owners" in branch protection (https://github.com/ossf/tac/settings/branches). We could also consider migrating from branch protection to rulesets (https://github.com/ossf/tac/settings/rules).

@SecurityCRob SecurityCRob merged commit 891331a into ossf:main Feb 5, 2024
3 of 4 checks passed
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.

5 participants