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

Remove engineering as a CODEOWNER #10

Merged
merged 1 commit into from
Aug 29, 2023
Merged

Conversation

NicholasEllul
Copy link
Contributor

Context

GitHub codeowners allow us to ensure that a specific group of people with write access are able to gate whether or not a pull request is allowed to be merged. It does this by adding them as a reviewer to every PR touching the designated files.

Problem

The problem with adding engineering as a codeowner is that each time a pull request in this repo is made, every member of our engineering team is hit with a notification. While codeowners make sense in cases where we want a specified group to gate changes, it becomes redundant where the code owning group is the entire engineering team.

Solution

Remove the code owner, and instead ensure that 1 approval is required before a pull request can be merged. This means that we still enforce a member of our engineering team to approve the pull request prior to a change being made, but now we are not sending review request notifications to everyone.

@NicholasEllul NicholasEllul requested a review from a team as a code owner August 29, 2023 17:58
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@NicholasEllul NicholasEllul merged commit b17e370 into main Aug 29, 2023
@NicholasEllul NicholasEllul deleted the ellul/change-codeowner branch August 29, 2023 18:13
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