How Google Performs Code Review
Critique and Gerrit
Google has two of its own code review tools: Critiqueused by most engineers, and Gerrit– open source, which continues to be used in public projects.
(You can experiment with Gerrit yourself in the open source repositories Chromium And Android.)
Dashboards
When engineers log in in the morning or take a break to review pull requests, internally called a change list, or CL, at Google, both Critique and Gerrit have dashboards that give them an easy, quick look at all the relevant changes (similar to the pull request window in a GitHub repository, only more complex and information-rich).
The Gerrit dashboard has a single search that retrieves information such as change size and more detailed CL status information (three columns on the right).
In the later Critique, the dashboard can consist of many sections, allowing authors to highlight different groups of CL (most often these are changes they created that have not yet been merged, and those that require their review).
Critique has one strange feature: a “Switch user” button in the top toolbar. It allows you to view another user's sections, for example if you want to make sure that your CL is actually visible to them.
View CL
When you click on CL, the following window opens:
Of course, there are the usual elements, such as title, description, diff, list of files and list of reviewers, but there are also additional useful elements that you won’t see on GitHub.
Gerrit displays test coverage metrics directly next to the file list:
The commit message for CL is displayed as a first-class viewable entity, located right next to the changed files in the diff section:
There are also some notable elements that are not visible or hard to see in the screenshot above:
Lines of code that were simply moved into the diff are visually displayed differently.
There are convenient quick features for standard replies: “The author of a change can click the “Done” button in the comment panel to indicate that the reviewer's comment has been taken into account, or the “Ack” button to acknowledge that the comment has been read; this is typically used for informational or support comments. Both of these buttons resolve the comment thread if it hasn't already. These quick features simplify the workflow and reduce the time it takes to review comments.” [1]
There are keyboard shortcuts for everything.
In addition to viewing the overall diff, you can also easily view CL diff versions. (Consider the situation: your colleague publishes CL(v0), you give feedback, he takes it into account and updates CL(v1), and now you only need to view what has changed, i.e. the diff between v0 and v1.)
Ready to merge?
In GitHub terminology, we usually say that a PR is ready to merge. However, Google says that CL is ready to submit. That is why “Submit Requirements” is present in the screenshots above.
However, before changes can be submitted to Google, they must go through three levels of mandatory approvals:
LGTM (looks good to me)
Code owners
Readability
The first two (LGTM and code owners) should be familiar to an engineer who has worked with GitHub.
LGTM can be approved by anyone; this means that the basic business logic is present in the CL (this is equivalent to the usual review approval of a pull request in GitHub).
Code owners clearly relate to the equivalent GitHub concept: when you make a change to a file, you need approval from whoever is listed as the owner of that file or the folder containing it. (If you are both the author and the owner, the approval occurs automatically.)
(In fact, GitHub borrowed the concept of code owners directly from Google; when GitHub added support for code owners in 2017, it included a small note at the bottom: “The code owners feature was inspired by Chromium's use of OWNERS files.”)
Readability is purely a Google concept.
IN Software Engineering at Google It describes wonderfully where it came from [2]:
In the early days of Google, Craig Silverstein (employee ID 3) would interview every new employee one-on-one and perform a line-by-line “readability review” of their first major code commit. This was a very thorough review, covering everything from possible ways to improve the code to whitespace standards. It gave Google’s codebase a consistent look and feel, but more importantly, it taught developers best practices by highlighting the existence of a common infrastructure and showing newcomers what it was like to write code at Google.” The readability program is still in use today, and is designed to maintain that standard among tens of thousands of developers merging tens of thousands of changes every day.
The point here is that it's not enough to just have someone take a second look at your code and understand its functionality. When tens of thousands of developers are committing code, you need to make sure that each commit matches detailed language standards and used the recommended patterns and libraries, so this requires an additional reviewer.
(As with code owners, if you are both the author and the “readability specialist” for the language you wrote the CL in, the change is automatically approved.)
“Attention groups” and “queues”
Since there could potentially be a bunch of different sets of reviewers per CL, for example, when working on another team's codebase you might need an LGTM from a colleague on your team (who knows best the change you want to make in your own product), code owner approval and readability reviews from other teams, it can be easy to become confused as to whose actions are required to approve a particular change.
Gerrit and Critique solve this problem perfectly.
In this screenshot from an older version of Gerrit, you'll notice that some authors or reviewers have a gray arrow next to their names.
And in Critique some names are highlighted in bold.
These markers indicate that it is someone's “turn” to take action on a particular CL. If you hover over the indicator, Gerrit or Critique will also tell you why they think it is your turn to take action (e.g. your review was just requested, or the author just responded to your comment). At any given time, the set of all the people whose turn it is now constitutes a common “attention set.”
For any CL, a reviewer can mark it as “not my turn”, removing themselves from the attention group and letting the author, and any other people remaining in the attention group, know who should actually take the action. This might be because, for example, their colleague is a more experienced reviewer, or because they want another reviewer to take the action first; in Google's culture, CL reviews are typically done first by the LGTM reviewer, followed by the code owner and the accessibility specialist.
Review Culture
When I first pitched this process to external engineers, their reactions were some combination of “wow, this looks like an awesome tool” and “this approval process seems pretty tedious.”
In 2018, several people from Google did the math and published the results in an article Modern Code Review: A Case Study at Google; It turned out that this process is not slow at all. [3]
“We found that at Google, the median developer authors make about three changes per week, and 80% of authors make fewer than seven changes per week. Similarly, the median developer reviewed changes is four, and 80% of reviewers review fewer than ten changes per week. We also found that the median developer waits less than an hour for feedback on their changes for small changes, and about five hours for very large changes. The overall median latency (across all code sizes) for the entire review process is less than four hours. This is significantly less than the median approval time reported by Peter Rigby and Christian 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 three Microsoft projects. Another study found that the median approval time at Microsoft is 24 hours.”
Part of the reason for such high speed is due to the tools – a dashboard and a clear queue indicating what exactly you need to pay attention. (There is also a handy Chrome extension that can render the main part of the dashboard, so you can look at it regardless of the page you are visiting.)
But Google's code review culture is just as important. All this specialized tools only confirms this. that Google cares more about code reviews than the average company.
When new employees join Google, they typically go through an onboarding period. During this period, reviews are more thorough and thoughtful than industry standards. Most early changes at Google typically take several days and endless rounds of feedback before the change can be merged.
This is also very much part of Google's code review process.
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 the 40th International Conference on Software Engineering: Software Engineering in Practice Track,nd https://sback.it/publications/icse2018seip.pdf.
