Preview Feedback: Require review from specific team rule

[patrick-knight]

Required review by specific teams now available as a preview feature in rulesets

Take control of your protected branches! You can now use rulesets to require approvals from specific teams for changes to specific files and folders.

What's new

• Protect critical code by enforcing strict reviews for your most sensitive files.
• Scale your policies across any repository, organization, or your entire enterprise.
• Target specific paths and require a set number of reviews from specified teams.

Rulesets vs. CODEOWNERS

While CODEOWNERS is great for defining ownership, this new ruleset focuses on policy enforcement. It makes it simple to require specific approvals on sensitive branches and critical code paths, all while scaling seamlessly across your enterprise.

This new rule is designed to augment CODEOWNER files but not replace them. The CODEOWNERS files will continue to be the way to manage ownership, support individuals as reviewers, and request reviews even if not required.

---

🌟 Leave a comment!

What do you think? Join the discussion and leave your feedback below!

[AlexCK-STFC]

Great in theory, but it doesn't seem negation works right now, when it says it uses the same syntax as .gitignore files!

I.e. this does not work:

**
!safefile.txt
!/safe-folder/**
!/safe-folder/

PRs are still blocked + require specific approval if a negated/allowed file is edited.

[patrick-knight]

Thanks for the feedback @AlexCK-STFC.

That's a miss on our end, the supported format is actually in line with fnmatch and will be rolling out a tweak to UI's language soon.

If we could support negations could you talk more about how that would fit in your workflow?

[AlexCK-STFC]

We are continuous deploying an app from configuration stored on GitHub

Most files are sensitive, and changes require specific approval from X members of a specific team.

Changes to a small number of files, some config files, are safe, and should be allowed to be merged quickly without requiring those team-specific approvals.

Ideally we'd use a wildcard, while negating a small number of non-sensitive files. Otherwise we'd need to keep updating a large list of files, which is easier through CODEOWNERS than through the repo settings.

The advantage of this new solution, over codeowners, is it allows control over the number of approvers from that team, which can be greater than the repos default.

[AndreasPetersen]

I haven't had the chance to try this feature yet, since it's still in preview, and I would need to ask my organization manager to enable it. Before asking them, I have a question though. It seems unclear from the announcement what the exact behaviour is for pull requests. Is it similar to how CODEOWNERS work?

Specifically I'm curious if this feature addresses https://github.com/orgs/community/discussions/178776. We would really like to be able to assign/require a team as reviewer, so that the team is automatically added to any PR as specified by the ruleset, but keep the team there as individual members add reviews to the PR.

[patrick-knight]

Yes this is similar to Codeowners in that the team will be added to the Reviewers sections of the PR.

I'm not certain about the second part of the question of the link shared is for this discussion but I presume it references something else.

[stevehipwell]

@patrick-knight should this be supported in the REST API?

[patrick-knight]

API docs for REST and GraphQL are coming soon.

[patrick-knight]

REST and GraphQL docs have be update to reflect this new rule.

[bkd365]

Great feature, our team is migrating from Azure DevOps and this covers a gap that codeowners didn't quite fill.

One issue is that for repository rulesets, I am only able to setup teams that have been given direct access to the repository. Teams with organization access to all repositories are not selectable. This limitation isn't there for rulesets created at the organization level.

[patrick-knight]

You can declare them on org rulesets, but are only enforced if the team has direct repo access, much like how bypass lists in repos allow you to define any bypasser even if doesn't have access to act on it.

This is a limitation currently in the pull request experience on the location teams can be sourced from.

[audunsolemdal]

Hey @patrick-knight - as you can see I've been interested in this for quite a while. After doing some testing, I think the current preview has potential but has some limitations compared to require review from codeowners which my org is currently relying on.

Specifically there are some cases where we require an approval for some paths for either team A or team B in CODEOWNERS
*.md @myorg/admin-team @myorg/project-team
I believe this is not possible with just using the new functionality(?) without CODEOWNERS - Hard for me to test as I don't have accounts to review/verify

Also the require review from specific team functionality is still one-dimensional in a sense. Ideally - say that I am a member of the team @myorg/admin-team I would like to have 0 required reviewers for **.tf files for this specific team. While members of other teams, e.g. @myorg/project-team should require an approval to make changes to **.tf files.

Extending on this - the functionality is still locked within Require a pull request before merging. There may be cases we would also like the option to bypass creating a PR entirely. I know this is a symptom of a larger issue, and I have a problem with this behavior in general, as it also causes issues in other contexts. Simply put - I think the logic for the checkbox Require a pull request before merging should be far more flexible than a true/false checkbox.

[patrick-knight]

The starting goals for us were around introducing branch specific reviewers which is hard to accomplish with CODEOWNERS, and add in discrete team based mandatory reviews.

The reviews for teams are ANDs unless you create a parent team that houses both sub-teams than the review can be satisfied by member of the broader team.

You can add multiple teams for the same paths, with different review requirements, so that should be doable.

We did recently move the CCR rule out of the PR rule into its own rule. I am curious to understand the desire to decouple review requirements from requiring pull requests

In the end this is why we launch previews to get feedback like this, so thanks for sharing.

[HughGrovesArup]

There doesn't seem to be any UI on the PR itself showing that this review is needed. Is this expected behaviour, or is my ruleset not being applied appropriately?

[patrick-knight]

I suspect there might be something misconfigured, as the reviews should show in the review section of the PR as well as inline in the merge box.

[HughGrovesArup]

I have reviewed and on new PRs the UI is showing, but on PRs that pre-date the rule being applied the UI is not showing.

[HughGrovesArup]

New PR:

Old PR:

[brendengoetz]

What do we need to do to be able to test this preview feature in our organization or repo? I am not seeing it available to enable/disable in my account under the "Feature preview dialog," nor do I see I way to request access to a technical preview.... I am a repo admin, but not an org admin if that matters. We very much are looking forward to this feature. Thanks!

[patrick-knight]

Hey @brendengoetz,

No need to request or enable this preview as it is available everywhere as a new option as part of the existing pull request rule.

[brendengoetz]

I see it now -- thank you!

[dustinbrooks]

@patrick-knight While not having the same functionality, we could control some exclusion logic with the order of patterns in CODEOWNERS, is that still true? Here is my scenario:
Team1 is responsible for Directory1 and everything in it.
Team2 and Team3 are responsible for everything else in the repo except Directory1.

In CODEOWNERS I just had Team2 and Team3 at the top with the "*": * @ORG/Team2 @ORG/Team3
And then the last line with /Directory1/ @ORG/Team1 took precedent and didn't include the other teams.

[dustinbrooks]

I was able to do a quick test just adding the rule and 3 reviewer teams.

For a change in Directory1 it added all 3 teams.

I would love to be able to just add Team2 and 3 with a * and exclude !/Directory1/ so we don't have to maintain an explicit inclusion list for everything, that would also allow a loophole I think for a new directory to get added and not require approvers.

[patrick-knight]

Thanks for the investigation and note, we've gotten the feedback on supporting exclusion/negations and will investigate feasible of that as part of this rule.

[Ant0wan]

Hi all :-)
I have been digging for few days now, but I cannot find any Terraform implementation of this. Is there a way to achieve it with IaC or gh-cli atm in order to automate it ?
Best,

[patrick-knight]

Not at the moment while we're still in preview. However it is available via the REST and GraphQL APIs

[gabrieltaylor]

Love this feature. We've been looking for a way to decouple ownership from review required.

I would like to humbly request an increase to the limit of 15 filter patterns per required reviewer. We're working with a pretty large monorepo and require quite granular rules. At the moment the highest number of patterns is 26 for one required reviewer but that may increase in the future.

Thanks for the amazing work on this feature.

[avivka]

Joining the good feedback and the suggestion to increase the limit. I'm actively working on an ADO -> GitHub Migration with few organizations that are reaching this limit quite easily @patrick-knight

[douglasg14b]

Subteams don't show up on the list of teams, which largely defeats the utility for us.

If we have a team my-domain and that then has subteams for each team within that domain, none of those show up, even though we already use those in our CODEOWNERS file without issue.

[gabrieltaylor]

@douglasg14b I found this too. You can work around this at the moment by granting the sub-team explicit access to the repo rather than the team inheriting access from their parent.