How to Grow a 10x Team with Code Reviews

by J. David Giese on October 25, 2021

This article explains why code reviews are essential for growing a 10x software engineering team. The first part of the article is theoretical, while the remainder uses this theory to make practical suggestions about using GitHub pull requests for code reviews. Please note that at Innolitics we write software for medical devices. Thus, although most of my suggestions are general, some may not apply to your situation.

Why do we do code reviews? 🔗

Why do we do code reviews? A high-level answer is “to provide value to our users and other stakeholders.” At Innolitics, we might say: “to mitigate safety risks, help patients, and make our clients profitable.”

Here are some lower-level answers:

We review code to catch bugs. That is, the author of the code meant it to act one way but actually acts another way. The reviewer might notice this and then it can be fixed.

We also review code to improve readability. Even if the code works, it can be difficult to understand. For example, the author may have used generic function names or their functions are very long. Code tends to make more sense to the author since they wrote it, so a reviewer can more easily spot poor organization or ambiguous names.

Good reviewers also check that the new code is consistent with the existing code’s architectural design. Reviewing design is more difficult and time-consuming since it requires a deeper understanding of the project.

It’s also good to check for tests. Are there enough of them? Do they test the right things?

You may want to check that all the stated requirements were met. It is easy to forget this step.

Your project may have other specific things to check for. For example, you may check that the documentation has been updated or that new risks have been documented and or mitigated.

Thus, we do code reviews for several reasons:

These are all good reasons to do code reviews, but I think they miss one of the most important ones: We do code reviews to improve the team!

{# TODO: discuss some of the downsides or possible problems with code reviews #}

Good code reviews improve the team 🔗

Good code reviews improve the team, and improving the team is the most efficient way to improve the code in the long run.

Why is this true? The short answer is that small improvements quickly compound. It’s the same logic that justifies investing in your development environment—incremental improvements in productivity pay for themselves quickly.

Someone may say: “I agree that if you can improve yourself one percent each day, you will become a 10x coder in no time! But are code reviews the most efficient way to improve yourself? Why not read Code Complete or watch online lectures instead?” We should use these resources too, but they tend to provide generic or theoretical advice. Code reviews give us concrete, nuanced situations to discuss—not the toy situations that books use due to lack of space and attention. Another benefit of code reviews is that a good reviewer will tailor their feedback to you.

Also, some improvements are project-specific, and won’t be found in books. In the least, code reviews familiarize team-members with the codebase. But deeper team-improvements can also occur. For example, the reviewer and the author may realize they’ve both been struggling with the same design problem, and then, through reasoned discourse, they come to a solution that neither was able to find on their own. Grace and I had a moment like this just the other week.

I don’t think code-reviews will automatically improve the team. It’s not a silver bullet. Code reviews are a lot of work. And to be effective the team must value learning and improvement and have mutual respect for each other.

Three circles labeled reviewer, author, and code---the code circle is big Three equally sized circles labeled reviewer, author, and code
We naturally focus our reviews on improving the code (left), but we should balance this goal with improving the team (right).

How code reviews improve the team 🔗

The observation that code-reviews improve the team has several implications. The most important is that in-depth code reviews are worth the time if the author and reviewer are improving themselves.

Without this realization, it may feel like time misspent to discuss whether a linebreak should be placed between a set of statements, or to debate two variations of a function name. However, the time investment can pay for itself and more during the rest of the project.

One of the reasons this is possible is because, if the author is improving, the length of the code reviews will decrease with time.

Code reviews can reduce skill-gaps.

In this chart, there is a skill-gap between the reviewer and the author. With each review, the skill gap shortens and so too does the review duration. I’ve drawn one larger review on the right. This is meant to represent an in-depth discussion that lead to a realization, thus bringing both the author and reviewer to the next level.

There are yet a few more ways that code reviews improve the team. In addition to seniors teaching juniors and equals teaching each other, juniors can also learn by reviewing a senior’s code and asking questions. If you think the main purpose of reviewing code is to improve the code, then a junior engineer reviewing their senior may not feel the need to do in-depth reviews. This is not the case. You should ask questions about the code you don’t understand.

Finally, I often learn a lot by reviewing junior’s code. Often I feel that a particular approach is off, but I can’t articulate why. Then, in the process of articulating my thoughts, I can better understand my thought process (and, sometimes, realize I’m mistaken or over-generalizing).

Thus, in conclusion, in-depth code reviews are a great way to improve the team, and thus the code.

Practice 🔗

GitHub pull requests 🔗

There are many ways to review code. E.g., you can email people a code-diff or you can do an over-the-shoulder review. At Innolitics, we usually use GitHub’s Pull Requests.

An idealized model of a pull request review.

In the practical part of this article I’ll discuss how to use GitHub pull requests. Some of these features include Draft PRs, reviewing by commit vs reviewing with the diff, sending individual comments vs a full review, the review status, and more.

When should you create a PR? 🔗

The first question an author needs to ask themselves is: When should I create the PR? There are several available options:

Sometimes you should request reviews before a feature is completed.

If you’re uncertain about your design, request a design review. Once all the code is constructed, changing the design can be expensive so the extra review is usually worthwhile.

Junior engineers should be especially quick to request design reviews. Also, project leads should suggest design reviews in the issue description if you think the design may be unclear.

How big should your PR be? 🔗

The next logical question is what should be included in a PR. Here are a few pointers:

Smaller PRs tend to be better. E.g., if you spend more than two weeks coding without a review, you should probably create a PR. Too many small PRs would be annoying, although I’ve only seen this happen a couple of times. People tend to err towards too big.

If you have an especially large PR and your reviewer is busier than you are, you can save them some time by squashing all the changes and creating a new set of logical commits. In essence, this creates a sequence of smaller PRs for the reviewer at the cost of your time and possibly some development history.

Who should you assign to the PR? 🔗

This is less of an issue for small teams since you’ll probably know who should review the code.

Senior engineers should review modifications that are higher risk. For example, on medical device projects built following the IEC 62304 standard, the software system is decomposed into smaller parts and each part is given a saftey classification of A, B, or C. Senior engineers should review the highest risk parts.

GitHub lets you indiate who should review specific files using the CODEOWNERS file. When combined with protected branches you can require that a code owner review PRs that touch that file.

If one of the purposes of code reviews is to level-up the team, then another consideration when assinging reviewers is who will learn the most.

It can also make sense to assign multiple reviewers, especially if one of the reviewers is junior or there is high-risk code.

What should you put in the opening comment? 🔗

When you create a pull request in GitHub, you must provide an opening comment.

Screenshot of GitHub's new pull request page circa July 2021.

What should you write there?

If comments will be useful long after the PR review, considering adding them to the codebase directly instead; answers to “why” questions or alternatives may make more sense in commit messages or comments.

PR comments are harder to track down then git commit messages or changes in the code. Also, comments in PRs are less attached to the code than comments or documentation. E.g., we’ve lost GitHub comments a few times in the past, e.g., when changing repository hosts or merging repositories.

Note: It can also be useful to write comments in your own code. E.g., to point someone to some code you’re worried about.

When should you push changes to an open PR? 🔗

When should you review? 🔗

Should you batch comments 🔗

GitHub let’s you send comments in batches or one-by-one.

Screenshot of GitHub's new comment user interface, circa July 2021.

If you begin reviewing the PR immediately after it is created, it’s often better to send comments one-by-one so the author can respond while you’re reviewing. It let’s them avoid a context switch. Also, you may be able to get through a couple rounds of back and forth responses on your earlier comments before you are done with the entire review.

How long should you spend? 🔗

We’re always busy with other tasks and PR reviews tend to be interruptions. Thus, it’s natural to spend too little time rather than too much. Furthermore, if the purpose of the review is just to improve the current code, an in-depth review may seem less worthwhile. However, if you keep in mind that code reviews improve the team, the cost is easier to justify.

Review all the life-cycle stages 🔗

One of the key ideas is to review all of the life-cycle stages. In the next few sections we’ll examine how to review each stage.

Review all life-cycle stages.

How to review requirements 🔗

The product owner is in the best position to review requirements, but it's still worth reviewing them as best you can.

Reviewing requirements is difficult because, unlike the code, we often don’t have the full picture. Really, only the product owner has the full picture. Thus, usually the best we can do is check that all of the stated requirements (stored in the GitHub Issue or the Software Requirements Specification) has been met. We can also review that the requirements gathering was properly performed. Some engineers, especially more junior engineers, have a tendency to guess the requirements. You can try to determine whether this occurred, and make suggestions as appropriate.

However, at the end of the day, our ability to review requirements is limited the most important take-aways from the article: We often use weekly sprint planning meetings. During the first half of these meetings we demo last week’s work. The main purpose of these demos is to have the product owner review our work and confirm that the requirements were met.

Occasionally you, as the reviewer, may remember some requirements detail that the author missed. This is most common if the project lead, who may have a deeper knowledge of the requirements, is the reviewer.

How to review requirements 🔗

How to review design 🔗

How to review construction 🔗

How to review tests 🔗

Use review checklists 🔗

Projects, especially regulated projects, probably have an explicit checklist in the development plan. We reccomend using GitHub’s saved-reply feature to streamline using review checklists.

Screenshot showing off GitHub's saved-reply feature, which makes it easy to follow a review checklist in your PR comments.

There are other things to check for here, including documentation.

When you’re feeling overwhelmed 🔗

Large pull-request reviews can be intimidating. Here are a few pointers

GitHub let's you mark certain files as "viewed." This is especially helpful for large reviews that require more than one sitting to complete.

How to give feedback 🔗

Negative results: “I was worried about your approach to X, but I read this post online about it and it made me think your approach is appropriate. I also read through the other code in detail, but I didn’t find anything to comment on. It looks great!”

Don’t not give advice, which you know to be good, just because you don’t always do it. Engineering is about tradeoffs. Sometimes you need to write code that is lower quality in one way to save time. But there are other code quality things which are less about saving time in the short term. If you are suggesting something that you don’t always do yourself, ask yourself why you don’t always do it? Maybe there is some tradeoff guiding your decision? Or maybe you don’t have a justification, and this is just an opportunity to put pressure on yourself to do better. Also, your situation may not be the same as the author’s situation. Likewise, as the author, if someone suggests something they don’t always do and you don’t think it’s necessary, then have a discussion exploring why the reviewer doesn’t always do it.

Review status and final comment 🔗

GitHub reviews can be sumbitted as a "comment", an "approval", or a "request for changes."

Here are some best practices for your final review comment:

Should I always leave comments? No. Sometimes there is nothing to say. It can be helpful to distinguish the case where you were too busy to do an in depth review from an in depth review that didn’t result in any comments. In the latter case, its helpful to comment on what you reviewed and any questions you asked yourself. For example, you could say:

If you just say “It looks great!” then the author may conclude you didn’t do a thorough review. As we discussed earlier, sometimes that’s okay, but you don’t want the person to think you were too busy or, worse, didn’t care, when in fact you left a careful review.

For example, if you recommends a technical book. Then, a few weeks later you get an email! “Thanks for the book recommendation; I read it and really enjoyed it!” You may wonder how carefully the person read it. If they read it carefully, wouldn’t they have said something more specific?

Don’t create issues where there isn’t anything to say. As you work with someone longer, the number of small comments will likely decrease. Eventually, most of the comments will be big-picture comments.

Responding to comments 🔗

Socrates: Thrasymachus, don’t be hard on us. If we are making any mistake in the consideration of the arguments know well that we’re making an unwilling mistake. If we were searching for gold we would never willingly make way for one another in the search and ruin our chances of finding it.

At Innolitics, we try to avoid absolute rules, but this is pretty close to being an absolute rule: respond to all PR comments.

Responding to comments. It is never acceptable to ignore comments entirely. This is disheartening to the reviewer. If you disagree with a suggestion, then make your case. If you think a refactor would make the code better, but isn’t worth the time investment, then make your case. Don’t ignore the comment. If there is enough time such that a review was performed, there is enough time to respond to the comments.

Fixing the issue vs responding: usually it is good to thumbs up, resolve, or write “fixed” in each PR comment. Why is just “fixing” the issue not sufficient? Sometimes there is a mismatch between what the reviewer thinks is being suggested, and what the author does. In this situations, one person may think they’ve addressed an issue while another person hasn’t

Responding to comments 🔗

Merging your PR 🔗

Conclusion 🔗

I hope you can use the theory and practices in this article to improve your team. Here are the most important take-aways from the article:

If you are building a medical device and would like help creating processes that comply with the FDA’s software guidance or IEC 62304 and also help you build quality software, we can help!

Get Medtech Software Tips

Subscribe using RSS

How frequently are they sent?

We send out tips about once a month.

What will I read?

Articles about software development, AI, signal and image processing, medical regulations, and other topics of interest to professionals in the medical device software industry.

You may view previous articles here.

Who creates the content?

The Innolitics team, and experts we collaborate with, write all of our articles.

Want to know more?

Contact us.