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 the option to not require a PR for protected branches #1359

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

Conversation

Kobzol
Copy link
Contributor

@Kobzol Kobzol commented Feb 28, 2024

This PR adds a pr-required = <true/false> option to branch protections. There are some repositories that use branch protections, but that do not require a PR.

I also refactored the branch protection mode to make the data representation clearer/make it impossible to represent invalid states (such as PR not required, but a required approval count is set). This is a compatibility break for the v1 data format, but branch protections are only read by sync-team anyway, so it shouldn't be a big deal.

An interesting thing is that it does not actually make sense to require any CI status checks if a PR is not required, because we do not enable anyone to bypass the branch protections. Because if you set a CI check, and try to push to the branch without a PR, it will be rejected.

Without a required PR, the branch protection still disallows force pushes and deletion, and allows us to set push actor allowances.

Relevant repos:

docs/toml-schema.md Show resolved Hide resolved
docs/toml-schema.md Show resolved Hide resolved
@Kobzol
Copy link
Contributor Author

Kobzol commented Mar 19, 2024

The sync-team side of this change is here.

@Kobzol Kobzol requested a review from rylev March 26, 2024 19:24
@@ -43,11 +47,15 @@
"branch_protections": [
{
"pattern": "master",
"ci_checks": [
Copy link
Member

Choose a reason for hiding this comment

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

This change breaks anything (namely sync-team) that relies on the current schema?

Don't think we can do this (at least not immediately).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that there's anything else than sync-team that read the branch protections, and the sync-team change is ready. I already did some breaking changes recently that broke sync-team, it doesn't seem like a big deal.

Although, it would be bad if this broke other tools that just reuse the schema, even if they don't read the branch protections at all. Not sure if there are any of these though. Is there even any other tool apart from sync-team that reads the repos endpoint?

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.

None yet

4 participants