According to Brook’s Law (from The Mythical Man-Month), “Adding manpower to a late software project makes it later.” Clearly coordination is hard, but for a growing company adding new engineers and helping them get productive is vital. For example, over the past year, the engineering team at Fullstory doubled in size while contributing thousands of code changes.
One way that we staved off chaos without compromising engineers’ autonomy was implementing a codeowners system—this ensures that the right people get eyes on any given change, without imposing major changes to our review process. GitHub has a built-in codeowners system, but for a team of our size the notification model became a huge impediment. Luckily, GitHub has a feature-rich API so judicious application of some bot logic can make GitHub codeowners work much more harmoniously!
How We Got Here
All development at Fullstory takes place in a single git repository, but (for now at least) we won’t wade into the well-trodden arguments for and against monorepos. We’re also serious about code reviews (as we’ve blogged about ). They educate new folks, spread knowledge, and, of course, catch bugs.
In the early days when there were only 6 engineers, our policy was simply “get a review from anyone else.” The codebase was small enough that everyone could mostly keep the entire thing in their head and thus could reasonably review most changes. And every engineer knew more or less what everyone else had worked on and thus who would be a good reviewer for any given change. With a small team, ownership is obvious and lines of communication are clear. Eventually the codebase grew varied enough, and the team grew large enough, that it made sense to have official “approvers” for different areas. We simply tracked in a google doc what these different areas were (we only had ~6 to start) and who were the approvers for each.
This worked “well enough” for a time but eventually outlived its usefulness, which we fully accepted last spring, as the team grew larger:
It was not very discoverable. New engineers wouldn’t always learn about this doc, or if they did it was easily forgotten.
It was easy to “violate” accidentally. The system relied on humans to figure out which approver groups were relevant for a given Pull Request (which might change over time as files are added or removed from the changeset!) and which humans would satisfy those approver groups.
It got stale over time. As the codebase grew and evolved we rarely updated the lists of files owned by each approver group.
Various details were not codified and thus were enforced inconsistently. For example, if the PR author is an approver in the required group, can they get an approval from
anyone or must they have an approval from someone else in the required group? Or, can one reviewer satisfy two different required groups (assuming they are a member of each)?
In designing a new system, our primary goal was Empathy, clarity, bionics: the Fullstory watchwords rather than regimentation. We view code owners as a way of helping to answer the question “who would be appropriate to review this code?” This is obviously valuable for newer engineers, who are still learning everyone’s names, much less what team everyone is on. But this is also useful for experienced engineers whenever they venture out into a part of the codebase they aren’t too familiar with. We very much encourage changes and contributions outside of your primary area of responsibility, but in order to make that work we have to ensure the right guidance from the right people.
Code owners address this bionically by letting us codify who is trusted to approve code in each area. Conveniently, GitHub provides built-in support for codeowners.
GitHub Code Owners: The Basics
GitHub codeowners is implemented as a single file (named, appropriately enough, CODEOWNERS) in the root of your repository. Much like a .gitignore file, each line of this file specifies a file pattern and one or more teams or individuals that “own” those files. Here’s an abbreviated example from the GitHub documentation:
Whenever a pull request is opened, GitHub will automatically check all changed files; for each that matches a codeowners rule, the owners are added as reviewers.
Furthermore, GitHub will enforce that the PR is not mergeable until all such reviewers leave an approving review.
For teams that are codeowners (as in the screenshots above) once any member of those teams submits an approving review, the entire review team is considered satisfied.
So what’s the problem? Aren’t we just done? Not quite…
Problems With GitHub Code Owners
When we first tried GitHub codeowners “out of the box” we ran into an issue that seemed minor at first, but quickly became a show stopper.
On GitHub when someone is set as a reviewer on a PR, they are automatically notified. This is useful and makes perfect sense if it’s a human assigning another human, but consider what happens when the code review system automatically assigns one or more teams as reviewers: every member of each of those teams is notified. Considering that PRs will generally only need a review from a single member of each team, and some of our approver teams have 10+ members, this creates a lot of needless noise and interruption.
Additionally, these notifications created confusion in the team. We use the Assignee field to request reviews from specific people, and many engineers on the team rely on notifications to know what PRs they need to review. Once these important emails were mixed in with the (relatively meaningless) emails from codeowner groups being auto-assigned, folks quickly started losing track of requested reviews, leading to longer review times and more manual reminders.
Using a GitHub Bot
After just a few days of this, we reverted our codeowner config and considered how we could write a GitHub bot to work around these issues. We did consider skipping GitHub codeowners entirely (making the bot do all of the work) but building off of codeowners meant that we wouldn’t have to reimplement the logic of “for every file changed in this PR, which approver team (if any) owns it?”
In our setup, there is still a GitHub team for each approver group, but they don’t actually include any humans at all—their one and only member is the bot. Instead, each group has a corresponding yaml file in the repo that lists all of the members of that group. This has the nice side-effect that changes to approver groups are just PRs, managed via standard code reviews, and the entire history is stored in git. In fact, we have now evolved a rough PR template for approver group additions, which includes lists of PRs authored and reviewed to show your mastery of that area of the codebase. Each team is a codeowner of its own approvers list and thus can “self-serve” adding new approvers whenever someone is ready to join. A special “approver-approvers” group can approve creation of entirely new approver groups.
Our codeowners file is just what you would expect, with file patterns mapped to approver groups
Let’s follow what happens when a PR is opened, in this case touching files owned by the “Product Experience & Workflows” (pew) and “Search Capabilities” teams. First GitHub will automatically compute which approver groups are required and add them as required reviewers:
Additionally, the bot comments on the PR reiterating the required groups along with the qualified approvers in each group:
The author will typically assign the PR to one person from each group. Whenever an approval is left on a PR, the bot looks at the approver and cross-references their identity with the approvers list for each required reviewer group. If one or more require reviewer groups are still unsatisfied, the bot will explain what still needs to be done. In this case, sbrubaker satisfies pew-approvers but not search-capability-approvers:
Once enough approvals have been left to satisfy all groups, the bot states its satisfaction and also approves the PR:
The bot’s approval is necessary to actually unlock the PR because the bot is the only actual member of these reviewer groups. In essence all of the prior approvals (by real people) were just a means to communicate to the bot so that it would know when it should approve on behalf of everyone.
Experiences and Future Work
Overall this system has served us quite well for over a year now. We’ve found it adds just the right amount of guardrails without becoming a burden, and it eliminates much of the ambiguity and inconsistency present with our manual, Google-Docs-based version. On occasion the bot can miss github webhook notifications (e.g. not realizing that someone has sent an approval) so we built a simple interface whereby leaving a special comment on a PR will trigger the bot to re-analyze the PR and either approve it (if appropriate) or leave a comment with detailed diagnostic information if not. This helps folks debug the occasional “why won’t the bot approve my PR?” question.
There are a number of enhancements that we have in mind for the future:
Automatically print out which files are covered by each reviewer group. Sometimes it is unclear why a particular group is required in a PR (especially if they own a single file with a small change that is easy to overlook). The only way currently to answer this is to scroll through the list of files in the diff view and hover over this subtle lock icon to display the codeowner for that file (which can be tedious in a large PR):
Smart assignments. Right now we count on humans to decide who specifically to assign. Especially when dealing with a different team from your own, it is hard to know who is best to choose (e.g. who is most familiar with that code, or who has the most time available right now, or who isn’t an approver yet but is actively looking for reviews to build up their resume). A bot could maintain a round-robin, or look at each person’s current review backlog, or implement any number of other useful strategies.
In June 2019, GitHub acquired PullPanda and now offers its tools for free. Included amongst these is Assigner, which “assigns code reviews to make your process more balanced and efficient”—which looks quite similar to our desire above for “smart assignments”! Interestingly (and coincidentally), their solution to “avoiding team notifications” involves setting up empty “proxy teams” in a manner very similar to Fullstory’s setup. Perhaps there are many organizations out in the world that have invented this very same workaround! ↩︎
