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

Feat/32 check branch protection rulesets #71

Merged
merged 20 commits into from
Mar 28, 2024

Conversation

AlexAxthelm
Copy link
Contributor

@AlexAxthelm AlexAxthelm commented Mar 19, 2024

For discussion in Tech Review this week (@hodie @RMI-PACTA/developers )

This PR adds an action to check for the existence of GitHub Rulesets, as well as some sample rulesets (in the rulesets/ directory)

Rulesets are the successor to "Branch Protection Rules" and "Required Checks". For our purposes, the most important part is that "Branch Protection Rules" do not apply to repo admins (by default: whoever created the repo), which rulesets do.

The workflow added here is intended to be called from another repo, and by default, it will check that the "Protect Main" rule is in effect, which:

  • Disallows deletion of main
  • Disallows force pushes to main
  • Disallows creating main
  • Disallows pushing to main
  • Requires Pull Requests to main have at least 1 approval, requires codeowner approval, dismisses stale reviews when new commits are added, and requires approval from someone other than the last committer to approve. (all roughly what we have now)

There is another ruleset example (not enabled by default), which requires certain R status checks to pass before the PR is eligible to merge.

@AlexAxthelm AlexAxthelm marked this pull request as ready for review March 19, 2024 15:29
Copy link
Member

@cjyetman cjyetman left a comment

Choose a reason for hiding this comment

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

I don't see anything glaringly, syntactically wrong here, but I think I don't fully understand how it works, so don't have much to say about it. If this is something that repo owners would implement in their repos by calling this action from an action in their repo, then maybe an example action (like other in this repo have) would be appropriate?

@AlexAxthelm
Copy link
Contributor Author

then maybe an example action (like other in this repo have) would be appropriate?

Yep. Planning on getting a number of the general/housekeeping checks ready together soon in a default/example action.

@AlexAxthelm AlexAxthelm merged commit 0cae5ea into main Mar 28, 2024
@AlexAxthelm AlexAxthelm deleted the feat/32-check-branch-protection-rulesets branch March 28, 2024 10:15
Copy link
Member

@jdhoffa jdhoffa left a comment

Choose a reason for hiding this comment

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

retroactively lgtm :-)

@MonikaFu
Copy link

MonikaFu commented Apr 5, 2024

does this require an action from me as a maintainer?

@jdhoffa
Copy link
Member

jdhoffa commented Apr 5, 2024

does this require an action from me as a maintainer?

No action required.

At this stage all actions in the actions repo are optional. This may change, however not before (many long drawn-out) discussions with the Tech Team.

Certain actions might be useful to you (e.g. https://github.com/RMI-PACTA/actions/blob/main/examples/R.yml and https://github.com/RMI-PACTA/actions/blob/main/examples/issues.yml)

but they are entirely at the discretion of the maintainer.

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.

4 participants