Most teams require code changes to be reviewed before they’re merged. A culture of high-quality code reviews helps engineers of all experience levels grow and promotes a shared understanding of the codebase. A poor code review culture inhibits innovation, slows down development, and builds resentment.

Your team will expect you to participate in code reviews—both to give and to receive them. Code reviews can bring out impostor syndrome and the Dunning–Kruger effect—phenomena that we discuss in Chapter 2. Both review anxiety and overconfidence are natural, but you can overcome them when armed with the right context and skills.

This chapter explains why code reviews are useful and how to be a good reviewer and reviewee. We’ll show you how to get your code reviewed and how to respond when you get feedback. Then, we’ll flip roles and show you how to be a good reviewer.

Why Review Code?

A well-executed code review is extremely valuable. There are obvious, superficial benefits—reviews can catch bugs and keep code clean—but a code review’s value goes beyond having humans stand in for automated tests and linters. Good reviews act as a teaching tool, spread awareness, document implementation decisions, and provide change records for security and compliance.

Code reviews act as a teaching and learning tool for your team. You can learn from the feedback that your code reviews get. Reviewers will point out useful libraries and coding practices that you might not be aware of. You can also read code review requests from more senior teammates to learn about the codebase and to learn how to write production-grade code (see Chapter 4 for more on writing production code). Code reviews are also an easy way to learn your team’s coding style.

Reviewing changes to the codebase ensures that more than one person is familiar with every line of production code. A shared understanding of the codebase helps the team evolve code more cohesively. Having others know what you’re changing means you’re not the only one the team can go to if things go wrong. On-call engineers will have added context about what code changed when. This shared knowledge means you can take a vacation without worrying about having to support your code.

Records of review comments also serve as documentation, explaining why things were done as they were. It’s not always obvious why code is written in a certain way. Code reviews act as an archive for implementation decisions. Having older code reviews to consult provides developers with a written history.

Reviews might even be required for security and compliance purposes. Security and compliance policies often prescribe code reviews as a way to prevent any single developer from maliciously modifying a codebase.

All these benefits of code reviews apply only when all the participants are able to work in a “high trust” environment, in which reviewers are intentional about providing useful feedback and reviewees are open to input. Poorly executed reviews become toxic impediments. Thoughtless feedback provides no value and slows developers down. Slow turn-around time can grind code changes to a halt. Without the right culture, developers can get into knock-down-drag-out disagreements that can ruin a team. Reviews are not an opportunity to prove how smart you are, nor are they a rubber-stamping bureaucratic hurdle.

Getting Your Code Reviewed

Code changes are prepared, submitted, reviewed, and finally approved and merged. Developers start by preparing their code for submission. Once code is ready, they submit the changes, creating a “review request,” and reviewers are notified. If there’s feedback, back-and-forth discussion occurs, and changes are made. The review is then approved and merged into the codebase.

Prepare Your Review

A well-prepared review request makes it easy for developers to understand what you’re doing and provide constructive feedback. Follow the VCS guidance that we give in Chapter 3: keep individual code changes small, separate feature and refactoring work into different reviews, and write descriptive commit messages. Include comments and tests. Don’t get attached to the code you submit for review; expect it to change, sometimes significantly, as it goes through the process.

Include a title and description, add reviewers, and link to the issue that your review request is resolving. The title and description are not the same as a commit message. The request’s title and description should include added context about how the changes were tested, links to other resources, and callouts on open questions or implementation details. Here’s an example:

Reviewers: agupta, csmith, jshu, ui-ux
Title: [UI-1343] Fix missing link in menu header
Description:

# Summary
The main menu header is missing a link for the About Us menu option. Clicking the menu button does nothing right now. Fixed by adding a proper href.

Added a Selenium test to verify the change.

# Checklist
This PR:

- [x] Adds new tests
- [ ] Modifies public-facing APIs
- [ ] Includes a design document

This request example follows several best practices. Both individual reviewers and the entire UI/UX team are added to the review. The title references the issue that’s being fixed (UI-1343). Using a standard formatting convention for issue references enables integrations that automatically link issue trackers with code reviews. This is helpful when referring to older issues later.

The description in the review also fills out a code review template that was included with the repository. Some repositories have a description template that gives reviewers important context about the change. A change that modifies a public-facing API might need added scrutiny, for example.

De-risk with Draft Reviews

Many developers think best by coding. Draft changes are a great way to think through and propose a change without investing as much time in writing tests, polishing code, and adding documentation. You can sanity-check what you’re doing by submitting a draft review: an informal review request intended to get quick and cheap feedback from teammates, which significantly reduces the risk that you go too far down the wrong path.

To avoid confusion, be clear when a code review is a draft or a work-in-progress (WIP). Many teams will have conventions for drafts; usually “DRAFT” or “WIP” is prepended to the title of the code review. Some code review platforms have built-in support for this; for example, GitHub has “draft pull requests.” Once your draft feels like it’s on the right track, you can transition it out of the “draft” state by finishing the implementation, tests, and documentation, and adding polish. Again, be clear when your code is ready for a nondraft review and then prepare the review request as described in the previous section.

Don’t Submit Reviews to Trigger Tests

Large projects often come with complex test tooling. It can be hard, as a new developer, to figure out how to run all relevant tests. Some developers bypass this problem by submitting code reviews to trigger the continuous integration (CI) system. This is a poor practice.

Submitting a code review as a way to trigger test execution is wasteful. Your review will fill the test queue, which will block reviews that actually need their tests to be run before merge. Your teammates might mistake your review request for something they should look at. The CI will run the full test suite, when you might only need to run tests related to your change.

Invest the time to learn how to run your tests locally. Debugging a failed test is easier locally than in CI environments; you won’t be able to attach debuggers or get debug information easily on remote machines. Set up your local test environment and learn how to execute just the tests you care about. Make your coding and testing cycle fast so you know immediately if your changes break anything. It’s an up-front cost, but it will save you time in the long run (and it’s friendlier to your teammates).

Walk Through Large Code Changes

Conduct code walk-throughs when making large changes. Walk-throughs are in-person meetings where a developer shares their screen and walks teammates through the changes that are being made. Walk-throughs are a great way to trigger ideas and get your team comfortable with changes.

Circulate relevant design documents and code in advance, and ask your teammates to take a look before the walk-through meeting. Give them adequate time—don’t schedule the walk-through for an hour later.

Start a walk-through by giving background about the change. A quick review of the design document might be warranted. Then, share your screen and navigate the code in your IDE as you narrate. Walk-throughs are best done by navigating through code flow from the start—a page load, API call, or application startup—all the way to the termination of the execution. Explain the main concepts behind any new models or abstractions, how they are meant to be used, and how they fit into the overall application.

Don’t try to get your teammates to actually review the code in the walk-through. Attendees should save their comments for the review itself. Walk-throughs are meant to help your team understand why a change is being proposed and to give them a good mental model for working through the code review in detail by themselves.

Don’t Get Attached

Getting critical comments on your code can be tough. Keep some emotional distance—the review is of the code, not of you, and it’s not even really your code; the whole team will own the code in the future. Getting a lot of suggestions doesn’t mean you’ve failed a test; it means the reviewer is engaging with your code and thinking about how it can be improved. It’s completely normal to get lots of comments, particularly if you are one of the less experienced developers on the team.

Reviewers might ask for changes that don’t seem important or that seem like they can be addressed later; they might have different priorities and timelines. Do your best to keep an open mind and understand where they are coming from. Be receptive to input and expect to revise your code based on feedback.

Practice Empathy, but Don’t Tolerate Rudeness

Everyone communicates differently, but rudeness should not be tolerated. Keep in mind that one person’s “short and to the point” can be another’s “brusque and rude.” Give reviewers the benefit of the doubt, but let them know if their comments seem off base or rude. If a discussion drags on or feels “off,” face-to-face discussion can help clear the air and get to a resolution. If you’re uncomfortable, talk to your manager.

If you disagree with a suggestion, try to work the disagreement out. Examine your own reaction first. Are you instinctively protecting your code just because you wrote it or because your way is in fact better? Explain your viewpoint clearly. If you still can’t agree, ask your manager what the next step is. Teams deal with code review conflicts differently; some defer to the submitter, others to a tech lead, and still others to group quorum. Follow team convention.

Be Proactive

Don’t be shy about asking others to review your code. Reviewers are often bombarded with code review and ticket notifications, so reviews can get lost on high-velocity projects. If you don’t get any feedback, check in with the team (without being pushy). When you do receive comments, be responsive. You don’t want your code review to drag on for weeks. Everyone’s memory fades; the faster you respond, the faster you’ll get responses.

Merge your changes promptly after you receive approval. Leaving a code review dangling is inconsiderate. Others might be waiting for your changes or want to change code once you merge. If you wait too long, your code will need to be rebased and fixed. In extreme cases, the rebase might break your code’s logic, which will require another code review.

Reviewing Code

Good reviewers break a review request into several stages. Triage the request to determine its urgency and complexity, and set aside time to review the change. Begin your review by reading code and asking questions to understand the context of the change. Then, give feedback and drive the review to a conclusion. Combining this recipe with a few best practices will substantially improve your reviews.

Triage Review Requests

Your work as a reviewer begins when you get a review notification. Start by triaging the review request. Some changes are critical and need to be reviewed right away. Most changes, however, are less pressing. If the urgency is unclear, ask the submitter. Change size and complexity also bear consideration. If a change is small and straightforward, a quick review will help unblock your teammate. Larger changes need more time.

High velocity teams can have an overwhelming volume of code reviews. You don’t need to review every change. Focus on the changes that you can learn from and those that touch code you are familiar with.

Block Off Time for Reviews

Code reviews are similar to operational work (discussed in Chapter 9); their size and frequency are somewhat unpredictable. Don’t drop everything you’re doing every time a review request arrives. Left unchecked, review interruptions can torpedo your productivity.

Block off code review time in your calendar. Scheduled review time makes it easy for you to continue on your other tasks, knowing you’ll have focused review time later. It’ll also keep your reviews high quality—you won’t feel as much pressure to get back to other tasks when you have dedicated time.

Large reviews might need additional planning. If you get a review that’s going to take more than an hour or two to get through, create an issue to track the review itself. Work with your manager to allocate dedicated time in your sprint planning session (see Chapter 12 on Agile development).

Understand the Change

Don’t begin a review by leaving comments; first read and ask questions. Code reviews are most valuable if the reviewer really takes the time to understand the proposed changes. Aim to understand why a change is being made, how code used to behave, and how code behaves after the change. Consider long-term implications of the API design, data structures, and other key decisions.

Understanding the motivation for a change will explain implementation decisions, and you might discover the change isn’t even needed. Comparing code before and after the change will also help you check for correctness and trigger alternative implementation ideas.

Give Comprehensive Feedback

Give feedback on a change’s correctness, implementation, maintainability, legibility, and security. Point out code that violates style guides, is hard to read, or is confusing. Read tests and look for bugs to verify code correctness.

Ask yourself how you would implement the changes to trigger alternative ideas and discuss the trade-offs. If public APIs are being changed, think about ways this may affect compatibility and the planned rollout of the change (see Chapter 8 to learn more about this topic). Consider ways in which a future programmer might misuse or misunderstand this code and how code can be altered to prevent this.

Think about what libraries and services are available that might help with the changes. Suggest patterns discussed in Chapter 11 to keep code maintainable. Look for OWASP Top Ten (https://owasp.org/www-project-top-ten/) violations, like SQL injection attacks, sensitive data leaks, and cross-site scripting vulnerabilities.

Don’t be overly terse—write comments the way you would say them if you were reviewing code sitting side by side. Comments should be polite and include both a “what” and a “why”:

Check that `port` is >= zero and raise an InvalidArgumentException if not. Ports can’t be negative.

Acknowledge the Good Stuff

It’s natural to focus on finding problems when reviewing code, but a code review doesn’t have to be all negative. Comment on the good stuff, too! If you learn something new from reading the code, mention that to the author. If a refactoring cleans up problematic areas of code or new tests feel like they make future changes less risky, recognize these things with a positive, encouraging comment. Even a code change you hate probably has something in it that you can say something nice about—if nothing else, acknowledge the intent and the effort.

This is an interesting change. I totally get wanting to migrate the queuing code to a third-party library, but I’m pretty averse to adding a new dependency; the existing code is simple and does what it needs to do. Definitely speak up if I’m misunderstanding the motivation; happy to talk more.

Distinguish Between Issues, Suggestions, and Nitpicks

Not all review comments have the same level of importance. Major issues need more attention than neutral suggestions and superficial nitpicks.

Don’t shy away from stylistic feedback, but make it clear that you’re nitpicking. A “nit” prefix prepended to the comment is customary:

Nit: Double space.
Nit: Here and throughout, use snake_case for methods and PascalCase for classes.
Nit: Method name is weird to me. What about maybeRetry(int threshold)?

If the same style issue occurs repeatedly, don’t keep harping on it; point out the first instance, and indicate that it’s something to fix across the board. No one likes to be told the same thing over and over, and it’s not necessary.

If you find yourself nitpicking style often, ask whether the project has adequate linting tools set up. Ideally, tooling should do this work for you. If you find that your reviews are mostly nitpicks with few substantial comments, slow down and do a deeper reading. Pointing out useful cosmetic changes is part of a review, but it’s not the main goal. See Chapter 3 for more on linting and code-cleanliness tooling.

Call out suggestions that seem better to you but aren’t required for approval by prefixing feedback with “optional,” “take it or leave it,” or “nonblocking.” Distinguish suggestions from changes you really want to see made; otherwise, it won’t necessarily be clear to the submitter.

Don’t Rubber-Stamp Reviews

You’re going to feel pressure to approve a review without really looking at it. An urgent change, pressure from a peer, a seemingly trivial change, or a change that’s too large will push you to sign off. Empathy might encourage you to turn a review around quickly—you know what it’s like to have to wait on a review.

Resist the temptation to rubber-stamp a review with a hasty approval. Rubber-stamping a review is harmful. Teammates will think you know what the change is and why it’s applied; you might be held responsible later. The submitter will think you have looked at and approved their work. If you can’t prioritize a review adequately, don’t review the change at all.

The temptation to rubber-stamp a request might be a signal that the code change is too big for one request. Don’t be afraid to ask your teammates to split up large code reviews into smaller sequential chunks. It’s easy for developers to get rolling and end up with a multithousand-line change. It’s unreasonable to expect a huge code change to be adequately reviewed in one shot. If you feel a code walk-through would be more efficient, you can also ask for that.

Don’t Limit Yourself to Web-Based Review Tools

Code reviews are usually handled in a dedicated UI like GitHub’s pull request interface. Don’t forget that code reviews are just code. You can still check out or download the proposed changes and play with them locally.

A local code checkout will let you examine the proposed changes in your IDE. Large changes are hard to navigate in a web interface. IDEs and desktop-based review tools let you more easily browse the changes.

Local code is also runnable. You can create your own tests to verify things work as expected. A debugger can be attached to running code so you can better understand how things behave. You might even be able to trigger failure scenarios to better illustrate comments in your review.

Don’t Forget to Review Tests

Reviewers will often gloss over tests, especially when the change is on the long side. Tests should be reviewed just like the rest of the code. It is often useful to start a review by reading the tests; they illustrate how the code is used and what’s expected.

Make sure to check tests for maintainability and code cleanliness. Look for bad test patterns: execution ordering, lack of isolation, and remote system calls. See Chapter 6 for a complete list of testing best practices and violations to look out for.

Drive to a Conclusion

Don’t be the reason improvements wither on the vine. Help review submitters get their code approved quickly. Don’t insist on perfection, don’t expand the scope of the change, clearly describe which comments are critical, and don’t let disagreements fester.

Insist on quality, but do not become an impassible barrier. Google’s “Engineering Practices Documentation” (https://google.github.io/eng-practices/) discusses this tension when reviewing a changelist (CL, Google’s internal term for a proposed code change):

In general, reviewers should favor approving a CL once it is in a state where it definitely improves the overall code health of the system being worked on, even if the CL isn’t perfect.

Respect the scope of the change that’s being made. As you read, you’ll find ways to improve adjacent code and have ideas for new features; don’t insist that these changes be made as part of the existing review. Open a ticket to improve the code and save the work for later. Keeping scope tight will increase velocity and keep changes incremental.

You can conclude reviews by marking them as “Request Changes” or “Approved.” If you leave a lot of comments, a review summary can be helpful. If you’re requesting changes, specify which changes are required to meet your approval. Here’s an example:

Change looks good. Few minor nits, but my main request is to fix the port handling. The code there looks brittle. See my comment for details.

If there is significant disagreement about the code change that you and the author cannot resolve, proactively propose taking the matter to other experts who can help resolve the disagreement.

Do’s and Don’ts

Level Up

Google’s “Code Review Developer Guide” at https://google.github.io/eng-practices/review/ is a good example of a company’s code review culture. Keep in mind that the guide is written specifically for Google. Your company’s tolerance for risk, investment in automated quality checks, and preference for speed or consistency might lead to a different philosophy.

At the end of the day, code reviews are a specialized form of giving and receiving feedback. The book Thanks for the Feedback: The Science and Art of Receiving Feedback Well by Douglas Stone and Sheila Heen (Penguin Books, 2014) is an excellent resource that will help you become both a better reviewer and a better reviewee.