Building an Inclusive Code Review Culture

Every engineer is familiar with code reviews, and many before us have compiled thoughts and suggestions on how to do them. Code reviews have an outsized impact on team culture because they are an integral part of the day-to-day of every engineer. At Plaid, we take pride in our inclusive and collaborative culture, and we felt that creating our own code review guidelines was one of the most impactful ways for us to grow that culture as we grow as an organization.

One of our four engineering values is Grow Together. This is a look at how we live that value through our code review process.

We approach code reviews with two key goals in mind: allow for learning and growth of all parties and ship healthy code. Plaid initially developed a collaborative code review culture because our early engineers practiced that. As the team continues to grow, we’ve realized that we can no longer take for granted the fact that every engineer has to have a direct relationship with every other engineer. We’d already figured out how to scale writing code itself, with linters that enforce specific style and style guides that articulate best practices. However, we hadn’t figured out how to scale the manner in which we are giving code reviews. We needed to condense the Plaid code review culture into something that could be distributed across the existing team and to new hires.

We started by documenting what the current best practices were at Plaid by going straight to the source: the entire engineering team. We interviewed a dozen engineers, ranging from new grads to our most senior engineers, and then compiled a doc that distilled everyone’s ideas into a handful of recommendations. We sent these out to the team to give everyone the opportunity to contribute and make their voice heard. After establishing agreement, we presented the following guidelines during a engineering all-hands to ensure alignment.

Plaid’s Code Review Guidelines

Goals of a Code Review

Beginning a code review with the right mindset is crucial to both giving and receiving feedback in a constructive manner. In most cases, our goals are to maintain the health of the code base, ensure correctness, and provide learning and growth for both parties.

For Authors

The most important rule for code authors is to be respectful of the reviewer’s time. Everything else stems from this principle. This means that when requesting a review, we should be sure that the code is ready for someone to take a look. That means making sure to have run the necessary tests, cleaned up any errant debugging statements, and have given the appropriate context in the review description. It’s important to ensure that whoever is looking at the code, whether that be the reviewer or a future engineer, can understand the motivations and how your code achieves that.

Carefully scoping your PR helps ensure an effective code review process. If you find yourself submitting hundreds of lines of codes for review, chances are you could be more rigorous in scoping the PR. Beyond a couple hundred lines of code, it becomes increasingly difficult for a reviewer to fully comprehend the changes and effectively review. By limiting changes, it also makes the code review process faster so that you can ship changes more quickly.

Finally, be respectful of the time that your reviewer has dedicated to helping you ship your changes. They’ve taken time from their core project work to give you feedback. If they leave comments, it’s important to respond no matter what, even if it’s to say you’ve made the change they suggested. Thankfully, we have an emoji for that: ⚡. It’s a great tool for tracking changes as well.

For Reviewers

One of the goals of a code review at Plaid is to maintain the health of our code base. Doing so is inherently fraught, because in many cases, suggesting ways to make code more readable or better fit style guidelines comes down to a matter of opinion. Thus, as the reviewer, it is important to be clear about what is opinion and what is fact. A bug in the code is objective, and should be called out as such. A different approach to decomposition or renaming a variable might make a world of difference in making the code readable, but it is still the opinion of the reviewer and should be communicated in that way.

At Plaid, we have style guides for each of the languages we use. If there is something you as the reviewer want to see in a codebase, for example, alphabetizing imports in typescript repos, that should go in the style guide. To codify global changes, engineers should make a PR against the style guide—not by going through every PR in every repo in that language. If someone has committed many crimes against the style guide in a PR, the reviewer should point them to the style guide with a mention of the specific violation and let the author fix it on their own as opposed to the reviewer going line by line and commenting on each violation.

Put another way, reviewers should avoid focusing on nits. A code review is not about going line by line and looking for fault in each line. A code review is about looking at the code as a whole to see how it fits into the larger system. That isn’t to say nit comments are never warranted, but they should not be the focus of the code review.

As a reviewer, you should always have an eye out for correctness. But a human will never be as effective at catching bugs as a computer. As important as it is for the reviewer to check for bugs, it’s even more important that they ensure the author has written proper tests for any new code paths introduced.

For Everyone

At Plaid, we have a "ship it" culture: With continuous deployments and employees around the world, shipping never stops. The nature of our work – integrating with almost 10,000 financial institutions – means that we’re constantly responding to changes in infrastructure. Thus, there’s often a sense of urgency with getting code reviewed. However, not all PRs are created with equal urgency, so it’s important to be upfront about timelines when asking for a review.

The process of code reviews allows for constant growth and learning, but only if you embrace this aspect of code reviews. Both authors and reviewers should be open to having a discussion and be willing to question why something is done a certain way. But it’s also important to recognize when a discussion has gone too far.

Nothing should be surprising to a reviewer in a code review: discuss any significant design decisions before code is written. Code reviews are a time to iron out implementation details, not discuss major design or architecture decisions. Sometimes things arise during code review that were not predicted to be points of contention but require significant discussion. More than a couple back and forths and the discussion should be taken offline and fleshed out in real time. This can be indicative of a PR that was not properly spec’d or scoped, but regardless of why a PR discussion has spiraled, it’s important to recognize that it has and solve it appropriately.

Implementing the Guidelines

In order to bring new hires up to speed and bring our code review recommendations to life, we’ve created a code review onboarding session that covers the guidelines we’ve created and includes a walkthrough in GitHub to look through recent PRs. By ensuring that new hires are exposed to the code review culture at Plaid, we believe it empowers them to contribute to that culture from the beginning.

For any new employee, it can be intimidating to try and get up to speed on all of the cultural norms of a new organization. But when learning cultural norms coincides with receiving critical feedback, as it does with code reviews, it can be particularly intimidating. For that reason, it's important to be as welcoming as possible when onboarding new hires into the code review culture.

At the end of the code review onboarding session you pick your LGTM emoji, which is a personal stamp of approval that you'll use to accept code reviews. Adding this emoji to the internal engineering README is a fun, symbolic way of starting your journey as a code reviewer at Plaid. We believe that welcoming new engineers as code reviewers from the very beginning is a key to strengthening our code review culture.

These guidelines will undoubtedly continue to evolve as our engineering organization evolves. Team members are encouraged to contribute to the guidelines so that they continue to be representative of our culture. While the specifics of these guidelines may change, the goal will remain the same: maintain an inclusive and collaborative engineering culture.

If this sounds interesting to you, consider joining our team!