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 localization team approvers (ar bn es pt ko) #432

Merged
merged 1 commit into from
Feb 17, 2022
Merged

Add localization team approvers (ar bn es pt ko) #432

merged 1 commit into from
Feb 17, 2022

Conversation

seokho-son
Copy link
Collaborator

@seokho-son seokho-son commented Feb 12, 2022

Based on l10n team initiation requests, I added localization approvers for each language.
(https://github.com/cncf/glossary/projects/2)

[Important]

  • All candidates need to leave a comment that you understood overall policy.
  • The l10n team approvers need to use your permission appropriately.
    • The approvers of l10n team will have a push permission to this repository.
      It is to make l10n approvers manage (merge) PRs for l10n contributions in your development branch.
      Merging a PR to the main branch by l10n approvers is restricted.
      Even if they are possible to review a PR to the main branch and give an approval to the PR, they should not approve the PR.
      So, please do not approve if a PR is not related with your localization.
  • After this PR merged, the approvers will get an github invitation for collaborate from settings[bot] of cncf/glossary. You need to accept.

[PR merge condition to main branch]

  1. only chairs can merge PR to main branch
  2. at least one of codeowners should give approving review
  3. at least 2 approving reviews are required from approvers.
  4. l10n team approvers can give approving review to PR for any file. (but they should not)

[PR merge condition to dev-xx l10n branch (dev-ko)]

  1. chairs can merge PR to all branches (caniszczyk, jasonmorgan, CathPag, seokho-son)
  2. at least one of l10n codeowners should give approving review
  3. at least 2 approving reviews are required from l10n approvers.
  4. l10n teams can merge PRs for their dev branch. If a PR includes files outside of l10n contents, They need to get an approval from maintainers (maintainers are codeowners for all files)

I hope to note that we are using the CODEOWNERS to automatically request a review to appropriate persons.
(https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners#about[…]owners)

The actual access permission can be configured from the repository setting (cncf/glossary repo).

@netlify
Copy link

netlify bot commented Feb 12, 2022

✔️ Deploy Preview for cncfglossary ready!

🔨 Explore the source changes: 12a2162

🔍 Inspect the deploy log: https://app.netlify.com/sites/cncfglossary/deploys/6207ed9295e2250008326a51

😎 Browse the preview: https://deploy-preview-432--cncfglossary.netlify.app

@seokho-son seokho-son added lang/ar for Arabic lang/bn for Bengali lang/es for Spanish lang/ko for Korean lang/pt for Portuguese maintainers Use this label if PR requires maintainers to take action labels Feb 12, 2022
@mitul3737
Copy link
Collaborator

Understood the responsibility properly @seokho-son 😁

@Mouly22
Copy link
Contributor

Mouly22 commented Feb 12, 2022

Understood the overall policy 😅 @seokho-son

@ikramulkayes
Copy link
Contributor

Got it @seokho-son. Understood the overall policy.

@electrocucaracha
Copy link
Collaborator

Understood the overall policy.

@raelga
Copy link
Collaborator

raelga commented Feb 12, 2022

Understood the overall policy.

🎉 💙

@same7ammar
Copy link
Collaborator

Understood @seokho-son 👍 .

@TarekMSayed
Copy link
Collaborator

Understood @seokho-son

@yunkon-kim
Copy link
Collaborator

Understood the overall policy 👍

@jessicalins
Copy link
Collaborator

Thank you @seokho-son! Understood the overall policy.

@AShabana
Copy link
Collaborator

Please accept me as an approver for Arabic language

@hacktron95
Copy link

Understood the policy, Thank you. 🚀

Copy link
Collaborator

@CathPag CathPag left a comment

Choose a reason for hiding this comment

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

Awesome!

@JasonMorgan JasonMorgan merged commit df93636 into cncf:main Feb 17, 2022
@seokho-son seokho-son deleted the up-permission branch February 17, 2022 02:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/ar for Arabic lang/bn for Bengali lang/es for Spanish lang/ko for Korean lang/pt for Portuguese maintainers Use this label if PR requires maintainers to take action
Projects
None yet
Development

Successfully merging this pull request may close these issues.