Critique and Gerrit
Google has two internal code review tools: Critique, which is used by the majority of engineers, and Gerrit, which is open-sourced and continues to be used by public-facing projects.
(You can actually play with Gerrit yourselves here in the Chromium and Android open-source repos.)
Dashboards
When engineers sign in in the morning, or take a break to review PRs — internally known as change lists or CLs — both Critique and Gerrit provide dashboards where its easy to see all of the in-flight changes at a glance (think a more sophisticated and information-dense version of the GitHub repository pull request view).
Gerrit’s dashboard (below) is backed by a singular search and pulls out information including the size of the change and a more detailed look at the status of the CL (the three columns to the right) to the top-level.
In Critique — which came later — the dashboard can have multiple sections allowing authors to highlight different groups of CLs (most commonly those that you’ve authored and haven’t merged and those that need your review).
One weird quirk of Critique: the “Switch user” button on the top toolbar. It allows you to view the sections of another user, e.g. if you want to make sure that your CL is actually on their radar.
CL view
When you click into a CL here’s what you’ll see:
Of course there’s your usual elements like your title, description, diff, list of files, and list of reviewers — but there’s also some additional useful features that you won’t spot on GitHub.
Test coverage metrics that appear right next to file list in Gerrit:
The commit message for the CL appears as a first-class, reviewable entity, located right next to the actual changed files in the diff section:
There are also a few notable items that either aren’t pictured or aren’t pictured well in the screenshots above:
Lines of code that have simply been moved in the diff receive a different visual treatment.
There are easy shortcuts for common responses: “The change author can click the "Done" button on the comment panel to indicate when a reviewer’s comment has been addressed, or the "Ack" button to acknowledge that the comment has been read, typically used for informational or optional comments. Both have the effect of resolving the comment thread if it is unresolved. These shortcuts simplify the workflow and reduce the time needed to respond to review comments.” [1]
There are keyboard shortcuts for everything.
In addition to seeing the overall diff, you can also easily diff versions of the CL. (Consider the scenario where your teammate posts a CL (v0), you leave a round of feedback, they address it and update the CL (v1), and now you want to see just what changed, i.e. the diff between v0 and v1.)
Ready to submit?
In GitHub parlance, we typically say a PR is ready to be “merged.” Inside Google though, you’d say that the CL is ready to be “submitted.” That’s why you’ll notice in the screenshots above there are “Submit Requirements.”
Before changes at Google are submitted though, they need to pass three levels of mandatory approvals:
LGTM (”looks good to me”)
Code owners
Readability
The first two of these — LGTM and code owners — will be familiar to an engineer using GitHub.
Anyone can give an LGTM which signifies that the core business logic of the CL checks out (equivalent to a regular approving GitHub PR review).
Code owners maps cleanly to the GitHub equivalent concept: when you make a change in a given file, you need to approval of whoever is listed as the owner for the file or containing directory. (If you are the author and are an owner yourself, you automatically pass.)
(In fact GitHub lifted the concept of code owners directly from Google; when GitHub added code owners support in 2017, it included a small footnote at the bottom: “The code owners feature was inspired by Chromium’s use of OWNERS files.”)
Readability, however, is a Google-specific concept.
Software Engineering at Google does a great job describing how this started [2]:
In Google’s early days, Craig Silverstein (employee ID #3) would sit down in person with every new hire and do a line-by-line “readability review” of their first major code commit. It was a nitpicky review that covered everything from ways the code could be improved to whitespace conventions. This gave Google’s codebase a uniform appearance but, more important, it taught best practices, highlighted what shared infrastructure was available, and showed new hires what it’s like to write code at Google.” The readability program remains in place today with the intention of continuing to uphold this standard across the tens of thousands of developers that merge tens of thousands of changes in each day.
The idea here is that it’s not just enough to have someone double-check and understand your functionality. With tens of thousands of developers committing code, you want to make sure that everyone is committing code that matches the lengthy language standards and is using the recommended patterns and libraries — so you add an additional reviewer for that.
(Like code owners, if you as the author are yourself a “readability expert” in the language you wrote the CL in, you’re automatically good to go here.)
Attention sets and turns
With multiple possible sets of reviewers per CL — working in another team’s codebase you may find yourself needing an LGTM from your teammate (who best knows the change you’re trying to make for your own product) and a code owner approval and readability review from others — it can become easy to lose track of whose action is needed to unblock a particular change.
Gerrit and Critique introduce a first-class treatment for this.
In the earlier Gerrit screenshot, you’ll notice that a gray arrow sometimes appears next to the name of an author or reviewer.
And in Critique, certain names can appear bolded.
These markers signify that it’s somebody’s “turn” to action on the given CL. Hover over the indicator and Gerrit or Critique will also tell you why it thinks it’s your turn to take action (e.g. your review was just requested or an author responded to your comment). At any given moment, the set of all the individuals whose turn it is create the overall “attention set.”
On any given CL, a reviewer can also mark themselves as “not my turn,” removing themselves from the attention set and clarifying to the author — and the others remaining in the attention set — who really needs to take action. This might be because their teammate is a better reviewer or because they need another reviewer to take action first; culturally, Google CLs are typically reviewed by the LGTM reviewer first, then the code owner and readability expert second.
Review culture
The common reactions I hear when engineers first learn about this process is some combination of “wow, that tooling sounds amazing” and “that approval process sounds tedious though.”
In 2018, a few folks at Google actually crunched the numbers and published their results as part of the larger Modern Code Review: A Case Study at Google paper — and the review process isn’t slow at all. [3]
“At Google, in terms of frequency, we find that the median developer authors about 3 changes a week, and 80 percent of authors make fewer than 7 changes a week. Similarly, the median for changes reviewed by developers per week is 4, and 80 percent of reviewers review fewer than 10 changes a week. In terms of speed, we find that developers have to wait for initial feedback on their change a median time of under an hour for small changes and about 5 hours for very large changes. The overall (all code sizes) median latency for the entire review process is under 4 hours. This is significantly lower than the median time to approval reported by Rigby and Bird [33], which is 17.5 hours for AMD, 15.7 hours for Chrome OS and 14.7, 19.8, and 18.9 hours for the three Microsoft projects. Another study found the median time to approval at Microsoft to be 24 hours.”
Part of this speed can be attributed to the tooling; the dashboard and its explicit turns that highlight what exactly you need to look at. (There’s also a handy Chrome Extension that can render most of your dashboard so you can peek at it regardless of the page you’re on.)
But, another equally important part is the culture at Google around code review. As all of this custom tooling testifies: Google just cares about code review more than the average company.
When folks first join Google, there’s usually an adjustment period. Reviews are more thorough and thoughtful than industry standard. Most Google’s first change usually takes multiple days and countless rounds of feedback before they can merge it.
That’s just as much a part of how Google does code review too.
Sources
[1] Written by Caitlin Sadowski, Ilham Kurnia, and Edited by Lisa Carey. “Critique: Google’s Code Review Tool.” Software Engineering at Google. https://abseil.io/resources/swe-book/html/ch19.html.
[2] Written by Caitlin Sadowski, Ilham Kurnia, and Edited by Lisa Carey. “Critique: Google’s Code Review Tool.” Software Engineering at Google. https://abseil.io/resources/swe-book/html/ch19.html.
[3] Sadowski, Caitlin, Emma Söderberg, Luke Church, and Michal Sipko. “Modern Code Review: A Case Study at Google.” Proceedings of 40th International Conference on Software Engineering: Software Engineering in Practice Track, n.d. https://sback.it/publications/icse2018seip.pdf.
