How I do Code Reviews

Nov 12, 2025

Over many years of reading other people's code I've developed a system for how I conduct code reviews.

Many people have praised me on my code reviewing skills, I'd like to share my approach so that others may benefit from my experience.

That's not to say that my approach is perfect. While I've been using this approach for over a decade, I still make occasional changes when I find room for improvement.

As a code reviewer my goals are as follows:

To be kind
To understand the problem being solved
To improve the code

Kindness is Key

The order of these goals is very important. As much as software engineering has a culture of direct and honest communication (e.g. the computer doesn't care about your feelings), we humans still have feelings and being unkind is antithetical to collaboration.

I hope this is obvious, but it's worth discussing anyway: being unkind or disrespectful to an author who worked hard on a feature may make them resentful and less open to criticism. It also means that they are less likely to put in hard work and effort in the future which will have an overall negative impact on the codebase. This means that kindness takes priority over code improvement.

Similarly, being unkind sets up an adversarial relationship where the author is less likely to share knowledge. Without sharing it's harder to understand the code, and understanding is important for avoiding mistakes. This means that kindness also takes priority over understanding.

As mere human reviewers we are prone to making mistakes. We may have misidentified an issue, misread the code, misspoke, linked to the wrong thing, commented on the wrong thing, misread the documentation, read incorrect documentation, or made any number of other mistakes that result in disagreement over the goal. If we seek to improve before we fully understand, we're liable to make more mistakes or add to the confusion. This means that understanding takes priority over improvement.

Clear Communication

There are many ways to miscommunicate during code reviews, so I like to establish conventions among my team so as to clarify expectations and unblock my teammates quickly. Vague comments like "I think this could crash the page" are relatively unhelpful without additional context.

I try to include the following information in code review comments:

priority
brief description of the issue or suggestion
links to supplementary information that the author might find helpful

Priorities

Priority includes a lot of context based on team conventions. I use a set of pre-defined priorities that each establish clear expectations for how to proceed with the code review.

When adding a code review comment. It's important to use the lowest applicable priority so as to give the author the most freedom to decide how to proceed.

Note

📓 (note)

I ran into a similar issue the other day on ABC-123.

The Note priority (using the :notebook: emoji) should be used for informational notes, or other commentary (jokes, memes) that don't require any changes from the author.

Code reviews with Notes may be approved.

Uncertain

❓ (uncertain)

What would cause the parameter to be undefined?

The Uncertain priority (using the :question: emoji) should be used for asking questions about the code. Sometimes follow-up conversations are needed for these comments in which case the author and reviewer are encouraged to document their findings before closing out the thread.

Code reviews with open questions may be approved at the reviewer's discretion.

Nitpick

💅 (nitpick)

Consider renaming the property from isDisabled to disabled for consistency with the other properties.

The Nitpick priority (using the :nail_care: emoji) should be used for small adjustments that are stylistic or otherwise unimportant to how the code functions. When authoring a nitpick comment, it's worth considering whether the comment is worth sharing at all.

Code reviews with open Nitpicks may be approved.

Low Priority

ℹ️ (low priority)

This looks right but we should add a test case for when undefined is passed as the second parameter.

Low Priority (using the :information_source: emoji) should be used for code recommendations that are unlikely to cause issue if they are postponed or skipped.

Code reviews with open Low Priority issues may be approved.

Medium Priority

ℹ️ (medium priority)

This looks like it may have a race condition if the second request fails before the first request is completed.

Medium Priority (using the :information_source: emoji) should be used for code that may cause issues if they are ignored. Often they may be postponed, in which case a new ticket should be created in order to address the issue separately. Linking the new ticket is sufficient to close a medium priority comment.

Code reviews with open Medium Priority issues may not be approved.

High Priority

⚠️ (high priority)

This field is missing a visible label, which is a violation of WCAG SC 3.3.2 Labels or Instructions.

High Priority (using the :warning: emoji) should be reserved for code issues that will definitely cause problems that can't be ignored or postponed.

Code reviews with open High Priority issues will be rejected.

Brief Description

The most valuable of all talents is that of never using two words when one will do.

— Thomas Jefferson

When working with professional software engineers, it's rarely necessary to go into great detail when reviewing code. Respecting your colleagues time and energy is a form of kindness.

I try to be brief and not mince words in code reviews, but it's important to be courteous and considerate for how feedback is given. Comments should always be about the code and never the author. Likewise, it's important to avoid casting judgement. Statements like "This code is bad" are an indirect attack on an author, and don't help the author move forward.

Finding the right balance can be challenging.

If I elaborate too much it can come off as patronizing.

If I don't say enough, it can come off as being conceited.

Authors should always feel welcome to reach out for more details when the writing is unclear, and reviewers should be prepared to make revisions on their feedback so that everyone can better understand the problem being solved.

Supplementary Information

Complex issues often require additioanl reading to fully understand the issue. Linking to external resources is a great way to share knowledge.

Sometimes the external resources don't exist yet. There may be internal conventions or discussions that are not publicly available that need to be referenced. In these cases it's beneficial to write up an external document of some sort. Ideally the document is shared in a way that it's easy to find by others on the team. Taking a little time to write up documentation and being intentional about where it's stored can pay dividends by avoiding the same issues in future code reviews.

Completing the Review

I primarily use GitHub's code review feature on Pull Requests, which has three options for finishing a review:

Approve: Approving the review should be done when there are no changes or discussions required. It is OK and expected that you may make many low priority suggestions and still continue to approve the code review.
Comment: Commenting on the review should be done when there are changes or discussions reqiured that are not high priority.
Reject: Rejecting the review should be used sparingly as it can block others from approving a PR. I try to use it only when high priority issues are unresolved.

I try to throughly review all code changes across all files. For small reviews this is not a challenge. Larger efforts that took the author longer to write merit taking longer on review. Skimming the review and commenting "looks fine" tells the author that their work wasn't worth your time.

In order to avoid dumping a lot of feedback all at once for larger reviews, I will sometimes complete code reviews in stages. Any time I need to submit an incomplete code review, I try to indicate that I have not yet finished reviewing the code. As much as possible I try to set expectations when further review is needed. It's better to let an author know whether you're most of the way done or just getting started so that they can plan accordingly.

When commenting or rejecting a review, I try to also summarize how many items need to be addressed, and remind the author that lower priority issues can be skipped if they're pressed for time.

I found 3 issues I'd like you to take a look at, along with a handful of unimportant nits.

And lastly, it's important to bring positive attention to the things you want to see more of. For example, if an author went out of their way to add detailed comments to make things understandable, let them know that you appreciate the effort.