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

ci: add required-review #3964

Closed

Conversation

evan-gray
Copy link
Contributor

The motivation for this PR is to allow for required reviewers which do not have write access to the repo. In order to achieve this, it leverages action-required-review.

In the short term this could co-exist with the full set of CODEOWNERS and eventually be the sole enforcer for much of the codebase, with the likely exception of the .github folder itself, or at least this action, which should probably remain protected by CODEOWNERS. The fallback could also remain with some users with write access if explicit approval from someone with write on every PR was desired.

Reviewers - I have attempted to replicate the existing CODEOWNERS rules in .github/required-review.yml, but please confirm they match.

The name field is optional, but I was leaning towards providing it for clarity.

Note that this config will match multiple entries, unlike CODEOWNERS which matches the last one. Therefore the ordering has been switched and some entries set to consume: true. See the Requirements Format for more details.

@evan-gray evan-gray force-pushed the required-review branch 2 times, most recently from e1a4d9d to 4ed0f7e Compare May 31, 2024 16:18
@evan-gray evan-gray marked this pull request as draft May 31, 2024 16:19
@evan-gray evan-gray force-pushed the required-review branch 2 times, most recently from d1575dc to 572ef52 Compare May 31, 2024 18:34
@evan-gray
Copy link
Contributor Author

evan-gray commented Jun 6, 2024

After further review, there are several shortcomings with a GHA actions approach, including

  • Users without write do not properly have their approvals dismissed
  • There are different status check results based on trigger making resolution difficult / clunky

Using rulesets to increase restrictions for users with write access is likely a better approach.

@evan-gray evan-gray closed this Jun 6, 2024
@evan-gray evan-gray deleted the required-review branch June 6, 2024 14:16
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.

1 participant