-
Notifications
You must be signed in to change notification settings - Fork 6.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
chore: Add CODEOWNERS for i18n reviews #2710
Conversation
PS: noticed that we have Catalan and Galician translations in here, but there aren't a corrisponding i18n team in the org |
Oh, I'm wondering if the teams used by the file might need to be given permission to the repo (even if it's reader), since in the reviewer dropdown you can find the website team, but not any other teams |
That was certainly true when we experimented in the core repo: nodejs/node#20554 (comment) (we ultimately removed it in nodejs/node#21161) |
Hmm, if protected branches is enabled here, giving write access shouldn't really matter (in this repo). Unless it was a different level of "write" access required |
@Trott would adding the i18n teams as writers to this repo require a TSC approval? |
Protected branches seems enabled but there's not a restriction in approvals AFAICT. It only requires CI to pass and branches to be up to date. |
I think if the website team broadly thinks it's a good idea and the i18n folks don't mind, then that should be OK. I'd notify TSC and CommComm for information purposes but I don't think there'd be any pushback. The number of people who have write permissions here is already pretty large. |
Maybe we'll land this and wait a few days or whatever to see if it works. If it doesn't quite work (which I expect), then we can move forward with plan "let's see if we can add the individual i18n groups to the repo". |
This was dictated from the docs https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners but I haven't really seen the
@repo/team
format work very well in other places, so maybe someone that's used this properly should take a look