-
Notifications
You must be signed in to change notification settings - Fork 214
Code Owners Configuration and PR Review Notification Settings
In this guide, we will cover how to configure Code Owners in agoric-sdk repository to allow external contributors to request reviews for pull requests (PRs) but not approve them. This setup is essential for maintaining a high level of code quality and ensuring that the right people are reviewing contributions. Additionally, Code Owners help external collaborators contribute effectively while adhering to project standards.
Implementing Code Owners provides several key benefits:
- Ensures code quality: Code Owners can ensure that only approved changes from trusted reviewers are merged into the repository.
- Streamlines collaboration: External contributors can confidently open PRs, knowing that the right team will be notified for a review.
- Automates reviews: When a Code Owner is defined for specific parts of the repository, GitHub will automatically request reviews from the right individuals or teams.
By setting up Code Owners, our repository will be better equipped to handle external contributions and maintain a high standard for incoming code.
To achieve this, we'll need to follow these steps:
The CODEOWNERS
file defines who is responsible for reviewing different parts of the codebase. It can be located in the following directories of master
branch:
- The root of the repository
- The
.github
directory - The
docs
directory
But we'll go for creating file inside .github folder
To create the file, add the following line to your CODEOWNERS
file:
* @Agoric/engprodexec
This will set the Agoric/engprodexec team as the default reviewer for all files in the repository.
Note: We can further add files/folders wise segregation as well.
More details here: GH Code-Owners
Once the CODEOWNERS
file is set up, the next step is to ensure that code owner reviews are required for merging PRs. This can be done by enabling the appropriate branch protection rules.
To configure branch protection rules:
- Navigate to Settings > Branches.
- Select the branch (e.g.,
master
in our case) and click Edit next to the branch protection rule. - Check the option for Require review from Code Owners.
We can directly visit the edit rules link for "master" branch (https://github.com/Agoric/agoric-sdk/settings/branch_protection_rules/17002382). Accessible to admin/owners of repo only.
This ensures that any PR touching the code owned by the specified team will require a review from them before it can be merged.
For Code Owners to approve PRs, they need write access to our repository. To grant this:
- Go to relevant team page (eg: https://github.com/orgs/Agoric/teams/engprodexec)
- Switch to Repository Tab (https://github.com/orgs/Agoric/teams/engprodexec/repositories)
- Add entry "Write Role" for
agoric-sdk
repo.
Note: We don't need to perform above steps as engprodexec
team has already the write access.
After getting done with code-owners configuration, we'll be having auto assignment of code-owners review on new opening PRs. This will create hassle and noise of "Review Requested" notifications to every team member. Is there any way to reduce that noise? Can we stop this notification while team-assignment? Let's get answer to these concerns in detail:
Unfortunately, GH doesn't provide such native notification settings where we could disable team review assignment notifications while keeping individual assignment active. Either we disable review assignment notifications completely or keep both (individual and team).
References: https://github.com/orgs/community/discussions/15747 https://github.com/orgs/community/discussions/15747#discussioncomment-5583613
So what can we do? Is there any workaround or hack?
The answer to this is YES. Let's see how:
- Have a dummy account and add it in the GH team
- Go to Team settings page (ed: https://github.com/orgs/Agoric/teams/engprodexec/edit/review_assignment)
- Check "Enable Auto Assignment" and "Only notify requested team members"
- Select "1" under "How many team members should be assigned to review?" option.
- Check "Never assign certain team members" and select all team members while keeping the dummyUser un-selected.
FINAL PICTURE:
So what have we achieved from above steps?
The dummyUser here is taking one for the team. Assignment/notification will only be sent to that dummyUser and the remaining code-owners will still be able to approve/review having their review the same weightage.
Here's filter to see the PRs assigned to your team:
team-review-requested:Agoric/engprodexec
External discussion on this hack: https://github.com/orgs/community/discussions/35673
If you're using GH slack app for notifications, you'll be having noise of "Team Review Assigned" alerts.
To avoid this noise, follow this recommended notification settings:
- Go to https://github.com/settings/reminders/Agoric
- Uncheck following options:
- "Review requests assigned to your team".
- "Your team is assigned a review"
Here's quick ecreenshot pointing out the relevant options
This wiki is for developing agoric-sdk. For help using Agoric SDK, see https://docs.agoric.com/ and https://agoric-sdk.pages.dev/