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

Fix configuration for branch protection #296

Merged
merged 2 commits into from
Jan 14, 2022
Merged

Fix configuration for branch protection #296

merged 2 commits into from
Jan 14, 2022

Conversation

seokho-son
Copy link
Collaborator

@seokho-son seokho-son commented Jan 6, 2022

The current settings for branch protection seems not work properly.
(I think some settings of https://github.com/apps/settings are not stable yet)

This PR updates .github/settings.yml and CODEOWNERS to fix configuration on branch protection.
I also suggest following configuration for permission.

  • All maintainers (admins) have push permission to all branches.
  • Approvers for each localization team have push permission to corresponding development branch (e.g. dev-ko)
    • but they cannot push a commit for files not related with localization contents unless an appropriate owner(reviewer) in CODEOWNERS gives an approving review.
    • they cannot push a commit to main branch

I'v tested https://github.com/apps/settings with a similar configuration in another organization :)
I hope this PR resolves all :)

Please note that

  • I temporally set myself as one of admin to check this setting by https://github.com/apps/settings works properly. (we can check this from setting tap)
  • I intentionally removed branch setting for Portuguese temporally. (Let's check configuration for Korean branch works firstly)

@netlify
Copy link

netlify bot commented Jan 6, 2022

✔️ Deploy Preview for cncfglossary ready!

🔨 Explore the source changes: 69f6bcc

🔍 Inspect the deploy log: https://app.netlify.com/sites/cncfglossary/deploys/61ddb90fe1c1390008bb51ba

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

@seokho-son
Copy link
Collaborator Author

seokho-son commented Jan 6, 2022

If this PR works, repository setting for branch will look like this.

image

@seokho-son
Copy link
Collaborator Author

@JasonMorgan PTAL this PR :)

@JasonMorgan
Copy link
Collaborator

Hey @seokho-son it looks good with the exception that @murillodigital is moving to emeritus status and shouldn't be included in maintainers/owners.

Copy link
Collaborator

@JasonMorgan JasonMorgan left a comment

Choose a reason for hiding this comment

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

Can we drop murillodigital from maintainers?

@seokho-son
Copy link
Collaborator Author

@JasonMorgan Thanks !
I updated codeowner and settings according to the emeritus status. ;)

@JasonMorgan JasonMorgan merged commit 6d32fb1 into cncf:main Jan 14, 2022
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.

2 participants