Code review is one of the cheapest ways to improve software, and one of the easiest to get wrong. When it works, bugs get caught before they ship, knowledge spreads across the team, and the codebase stays consistent. When it doesn’t, pull requests sit for days, reviewers argue about formatting, and authors learn to dread the notification. I’ve already written a short guide to code review best practices. This post is the long version. It covers what review is for, what needs to be in place before it can work, the human side that most guides skip, practical steps for authors and reviewers, what to look for in the code, and which parts to hand to tools.

What code review is for

Code review means that one or more developers read a change before it’s merged. Most teams do it through pull requests on GitHub, Azure DevOps, GitLab or Bitbucket. The obvious reason to review is to find bugs, and reviews do catch them: logic errors, missed edge cases, security holes. But when Alberto Bacchelli and Christian Bird studied code review at Microsoft in 2013, they found that while developers said finding defects was their main motivation, much of what reviews actually produced was something else. Reviews spread knowledge of the codebase, gave the team alternative solutions, and made the code easier to understand. A reviewer who learns how the payment module works is the person who can fix it next month when its author is on holiday.

Review also keeps a codebase consistent, because every change passes through someone who knows the team’s conventions. And it’s cheaper than the alternative. A problem found in review costs a comment and a small fix, while the same problem found in production costs an incident.

The standard a reviewer applies matters as much as the review itself. Google’s engineering practices put it well: approve a change once it definitely improves the overall health of the code, even if it isn’t perfect. A reviewer who holds every pull request until it matches the code they would have written slows the team down and teaches authors to avoid them.

Kinds of code review

• Asynchronous pull request review is the default today. The author opens a pull request, reviewers comment in the diff, and the author updates the change until it’s approved.
• Pair and mob programming review the code while it’s being written. A second person sees every line as it happens, so the separate review step can be short, or skipped for low-risk changes.
• Formal inspections, which go back to Michael Fagan’s work at IBM in 1976, use defined roles, checklists and a meeting. They’re still used for safety-critical code, and they’re too heavy for most product teams.
• Ship/Show/Ask, described by Rouan Wilsenach on Martin Fowler’s site, lets the author choose per change. Small, obvious changes are merged directly (“ship”). Some are merged and then shown to the team for comments (“show”). Only the ones that need discussion wait for approval (“ask”). That keeps blocking reviews for the changes that need them.

What has to be in place first

Most code review problems aren’t about how people comment. They come from missing foundations: no time set aside for reviewing, no clear requirements, no agreed standards. Fix these first.

Time and priority

Reviewing takes time, and that time has to be planned and protected. Managers need to treat review as part of the work, not as a favor done between tasks. If a team is measured only on what each person ships, reviews will always come last. In practice, that means reviewing before starting new work, and responding quickly. Google’s guidance is one business day at most, and preferably much less. A pull request that waits three days forces the author to switch back into a context they’ve already left, which is where most of the cost of slow reviews comes from.

Not every pull request is equally urgent. A fix for a production bug, or a change that’s blocking a teammate, comes before a refactoring that can wait a day. Agree as a team how urgent changes are flagged, so reviewers know which pull request to open first.

Clear requirements

A reviewer can only judge whether code is correct if they know what it’s supposed to do. Every pull request should link to a ticket with acceptance criteria. Without one, the review turns into a debate about what the feature should be, which is a conversation for the ticket, held before anyone writes code.

Coding standards and automation

Arguments about brace placement and naming waste review time and build resentment. Agree on a coding convention, enforce it with a formatter and analyzers, and run them in CI. Reviewers then never need to comment on style, because the build already has.

Written review guidelines

Write down how review works on your team, so new members don’t have to guess:

• how many approvals a pull request needs
• who must review which areas (a CODEOWNERS file automates this on GitHub, GitLab and Bitbucket)
• which kinds of comment block a merge
• what the author must do before asking for review
• how fast reviewers are expected to respond

A short checklist of things reviewers should always check, such as error handling, tests and security-sensitive changes, helps less experienced reviewers know where to look. Add the author’s pre-review steps to your Definition of Done, so the expectations are the same for everyone.

Where the conversation happens: the pull request, not chat

Keep review discussions on the pull request. Comments there are attached to the exact lines they’re about, visible to everyone on the team, and kept with the change for good. Six months later, when someone wonders why the code works the way it does, the reasoning is one click away from git blame. A conversation in Slack or Teams is quicker, but it’s scattered across channels and threads, half the team never sees it, and it’s gone when someone needs it later.

Chat and calls still have their place. Sometimes a five-minute conversation settles what twenty comments couldn’t. When that happens, post a short summary of the decision back on the pull request, so the record stays complete.

The human side of code review

Code review means telling a colleague what’s wrong with their work, often in writing, without tone of voice to soften it. How that’s done decides whether people look forward to reviews or dread them. These habits help.

For reviewers

• Comment on the code, not the person. “This function doesn’t handle an empty list” is about the code. “You forgot the empty case again” is about the author.
• Ask questions when you’re not sure. “What happens if items is empty here?” invites an answer, and sometimes the author has already handled it somewhere you didn’t see.
• Explain why. “Use a guard clause here” is an instruction. “A guard clause here would remove a level of nesting and make the happy path easier to follow” is a lesson the author can apply next time.
• Be specific, and show what you mean. “This is confusing” gives the author nothing to act on. A short example of the alternative, or a suggested change using the platform’s suggestion feature, does.
• Say which comments block the merge. Labels such as Conventional Comments make the weight of each comment explicit:

issue: If `items` is empty, `items[0]` throws. Can we return early before this line?

question: Why is the retry count 5 here and 3 in PaymentClient? If there's a reason, a comment would help the next reader.

nitpick (non-blocking): `orderTotal` would read better than `ot`.

praise: The guard clause makes this much easier to follow than the old version.

• Point out what’s good. Praise for a clean solution or a well-written test costs nothing. It also tells the author which habits to keep, which is as useful as telling them what to change.
• Take disagreements to a call. If a thread has gone back and forth twice without agreement, talk it through, then write the outcome on the pull request. Long written arguments tend to become personal.

For authors

• Assume good intent. A blunt comment is usually a rushed one, not an attack. Read it as being about the code.
• Treat feedback as information. People are much less defensive when they see feedback as a chance to learn (what Carol Dweck calls a growth mindset) than when they see it as a judgment of their ability. That attitude is easier when it’s shared across the team, and senior developers set it by how they react when their own code gets comments.
• Explain your reasoning when you disagree. The reviewer may be missing context, or you might be. Either way, the discussion should end in a decision both of you understand.
• Reply to every comment. A fix, a short reply, or “done” all tell the reviewer the comment was read.

For authors: before you ask for a review

Most of what makes a review fast is done by the author before anyone else looks at the code:

• Keep the change small. In a well-known study of code review at Cisco, published by SmartBear, reviewers found defects less effectively once a review went past about 400 lines of code, or past about an hour of reviewing. Split large features into a series of pull requests (refactor first, then the new behavior), or hide unfinished work behind a feature flag. If a review would be unmanageable, the reviewer is entitled to ask you to split it.
• Review your own diff first. Read the pull request in the review UI the way a reviewer will. You’ll catch leftover debug code, unrelated changes and missing tests before anyone else has to point them out.
• Make sure it works. The tests pass locally and CI is green. Run the feature by hand if it has a UI.
• Write a description. Say what changed, why, and which ticket it belongs to, how to test it, and what deserves a careful look. A pull request template makes this a habit:

## What and why
<!-- What does this change do, and which ticket does it belong to? -->

## How to test
<!-- Steps, or the tests that cover it -->

## Risks
<!-- Migrations, feature flags, anything that needs a careful look -->

## Screenshots
<!-- For UI changes -->

• Use a draft pull request for early feedback. If you want an opinion on the approach before you finish, open the pull request as a draft and say so. Don’t send unfinished code for a full review without saying it’s unfinished.
• Respond promptly. A review that waits days for your reply costs the reviewer the same context switch that a slow review costs you.

For reviewers: how to review a pull request

1. Read the description and the linked ticket first, so you know what the change is meant to do before you look at the code.
2. Check that CI is green. If the build or the tests fail, ask the author to fix that before you spend time reviewing. There’s no need to build every pull request yourself, because that’s CI’s job.
3. Look at the overall shape before the details. Does the approach make sense, and are the changes in the right places? If the design is wrong, say so now, before you leave 30 comments on lines that will be rewritten anyway.
4. Read the tests early. They show what the author thinks the code should do, and missing cases stand out quickly.
5. Go through the diff file by file. When reading isn’t enough, check out the branch (git fetch, then git switch <branch>) and run it locally, for example for UI changes or for code that’s hard to follow without stepping through it in a debugger.
6. Leave comments in the pull request, labeled so the author knows which ones block the merge.
7. Approve, or request changes. When the author pushes fixes, review only the new commits. Most platforms can show the changes since your last review.
8. Merge with the platform’s merge button, not a local git merge, so branch protection rules and required checks apply. Let the platform delete the branch afterwards.

What to look for

Functionality

Does the code do what the ticket asks, including the edge cases? Check empty inputs, nulls, boundaries, errors from dependencies, concurrent access and time zones. Think about users: will this change surprise anyone, and does it meet any compliance requirements your domain has? Bugs found here are the most valuable part of a review.

Design and structure

Is the change in the right layer, and does it fit the existing architecture, or is it working around it? Look at separation of concerns, coupling and cohesion, and duplicated logic that should live in one place. A new dependency deserves a closer look than any line of code: whether it’s maintained, its license, its size, and whether it’s needed at all. Check that the design follows the team’s established patterns. When a pull request introduces a new pattern, that’s a decision for the team, not just for this pull request.

Readability and maintainability

Could someone new to this code understand it six months from now? Check that names say what things are for and that functions are small enough to follow. Comments should explain why, not repeat what the code says. Static analysis measures complexity, such as cyclomatic complexity or nesting depth, but the real test is whether you, as the reviewer, had to read a function three times to understand it.

Tests

Do the tests cover the new behavior and the edge cases, and would they fail if the change were reverted? Coverage numbers show what code was executed, not whether anything was checked. A test that runs the code and asserts nothing reaches 100% coverage and proves nothing. Look for the right level of testing. Unit tests should cover logic in isolation, integration tests should cover the boundaries with the database and other services, and a few end-to-end tests should cover the main user flows. Check that test names describe behavior, and that the tests don’t depend on timing or on each other.

Security

Watch for:

• input that isn’t validated
• SQL or commands built from strings
• output that isn’t encoded
• authorization checks that confirm the user is signed in but not that they may access this particular record
• secrets in code or configuration files
• sensitive data written to logs
• dependencies with known vulnerabilities

The OWASP Top 10 is a good checklist for what to look for, and my security best practices post covers the .NET and React specifics.

Performance

Look for the problems that are cheap to fix now and expensive later:

• database queries inside loops (the N+1 problem)
• queries with no paging or limit
• blocking on async code
• large allocations in code that runs on every request
• algorithms that are O(n²) over data that will grow

Don’t ask for micro-optimizations without a measurement behind them. A slower but clearer solution is usually the right call until a profiler says otherwise.

Documentation

Public APIs should have doc comments. The README, the OpenAPI description and the changelog should be updated when behavior changes. A non-obvious decision deserves either a comment in the code or a short architecture decision record.

The pull request itself

Is it focused on one thing, or does it mix a feature with unrelated cleanup? Do the commit messages make sense? Is it linked to its ticket? A pull request that’s too big to review properly should be split, and saying so is part of the review.

Let the tools do what they can

Every issue a tool catches is one a reviewer doesn’t have to find. Run these on every pull request in CI, and make the important ones required checks:

• Build and tests. A pull request with a red build isn’t ready for human review. Continuous integration also keeps branches short-lived and merges small, which makes each review easier.
• Formatters: dotnet format or CSharpier for C#, and Prettier for JavaScript and TypeScript. They end every formatting discussion.
• Analyzers for C#:
  • The .NET analyzers (Microsoft.CodeAnalysis.NetAnalyzers) ship with the .NET SDK and run during every build, not just in Visual Studio.
  • StyleCop.Analyzers adds style and documentation rules.
  • ReSharper and Rider’s inspections catch code smells while you type.
• Linters for JavaScript and TypeScript: ESLint with typescript-eslint and the security plugins. JSHint has largely been replaced by ESLint.
• Platforms: SonarQube and SonarCloud cover many languages and track duplication, complexity, coverage and security hotspots over time. GitHub’s CodeQL scans for vulnerabilities, and Dependabot or Renovate keep dependencies current.
• AI review assistants: GitHub Copilot code review and similar tools can leave a first round of comments within minutes, and they’re good at spotting missed null checks and inconsistent naming. Treat their comments like any other suggestion: some are useful and some are confidently wrong. A human still approves the merge. With more code being written by AI, careful human review matters more, not less.

Measure the process, carefully

A few numbers show whether review is helping or slowing the team down:

• time until a pull request gets its first review
• total time from opening to merging
• pull request size
• how many pull requests are waiting at any given time

If the first review routinely takes more than a day, or most pull requests are over a thousand lines, those are the problems to fix first. Never measure individuals by comments per review or approvals per week. People will optimize for the number, and the reviews will get worse.