openskills.info

Code Review

itSoftware engineering

Code Review

Code review is a structured examination of a proposed code change by someone other than its author. The reviewer studies the change before it joins the shared codebase. The author responds by revising the code, explaining a decision, or discussing an alternative.

The review is not a search for a flawless change. Its purpose is to decide whether the change improves the codebase while meeting its intended goal. A useful review also spreads knowledge and records important reasoning.

Most teams perform code review through a pull request or a similar change-list tool. The tool shows a diff, which is the difference between the proposed code and its base version. It also keeps comments, revisions, checks, and the final review decision together.

Why review code

Automated checks can compile code, run tests, enforce formatting, and detect known patterns. They are fast and repeatable. They cannot fully judge whether a change solves the right problem, fits the surrounding design, or handles business rules correctly.

A human reviewer adds context. You can compare the implementation with the stated intent. You can inspect behavior that the tests omit. You can notice unnecessary complexity, unclear names, unsafe data flow, or a design that makes the next change harder.

Review also creates a second point of ownership. The author knows how the change was built. The reviewer learns why it exists and how it fits. That shared understanding matters when the code later fails or needs modification.

The unit of review

A code review begins with a proposed change. On GitHub, that proposal is a pull request. Other tools may call it a change list or merge request. The name differs, but the working unit is the same: intent, code, tests, discussion, and a decision.

The author makes the review possible. A useful proposal has a clear purpose, enough context to understand the problem, and a focused set of changes. Unrelated formatting or refactoring can hide the functional change. Very large proposals also make it harder to find the main design decision and review every relevant line.

The reviewer supplies independent judgment. Independence does not require distance or hostility. It means you verify the change instead of assuming that its implementation proves its correctness.

Read in layers

Start with the purpose. Read the description, linked issue, design notes, and relevant requirements. Ask what behavior should change and what must remain unchanged. If the purpose is unclear, request context before spending time on line-level details.

Next, find the center of the change. Inspect the main behavior and its design before reviewing minor files. A fundamental design problem can make detailed comments obsolete. Tests can be a useful early read because they show the behavior the author expects.

Then inspect the complete diff and enough surrounding code to understand it. A diff shows what changed, but the unchanged context still determines whether the change belongs there. Review every human-written line in your assigned scope. State your scope when several reviewers split the work.

Finally, form a decision. Separate defects that block approval from optional improvements. Summarize the important findings so the author can act without reconstructing your intent from scattered comments.

What to examine

Begin with correctness. Check whether the implementation matches the intended user or system behavior. Follow important paths through inputs, state changes, outputs, and failures. Look for boundary conditions and assumptions that the code does not enforce.

Examine design and complexity. Ask whether the change fits existing responsibilities and whether a smaller design would work. New abstractions need a current purpose. Speculative flexibility can increase maintenance cost without helping the present change.

Review tests as production assets. Confirm that the changed behavior has appropriate tests. Check whether the assertions would fail for the relevant defect. Tests also need clear names, useful setup, and maintainable structure.

Inspect readability. Names should communicate purpose. Comments should usually explain reasoning that the code cannot express. Documentation should match the new behavior. A future reader should not need the review conversation to understand the final code.

Apply the project's established standards. Formatting and other mechanical style rules belong in automated tools when possible. Do not block a change on a personal preference that the team has not adopted.

Include security where the risk requires it. Trace untrusted input, authorization decisions, sensitive data, and failure behavior. Manual secure review complements automated security tools because business logic and context-specific flaws often require human judgment.

Write comments that lead to action

Comment on the code, not the person. State the observed problem, explain why it matters, and distinguish a required change from a suggestion. The author remains responsible for choosing and implementing the fix unless a precise suggestion is the clearest help.

A strong blocking comment might say: "This path authorizes the account from the request body. Please derive the account from the authenticated session so one user cannot select another user's account."

A weak comment might say: "This is wrong."

The strong version names the behavior, the risk, and the required outcome. The weak version provides no evidence or direction.

Use a consistent severity vocabulary. For example, label comments as blocking, suggestion, question, or minor. The exact labels matter less than a shared meaning. An author should never have to guess whether approval depends on a comment.

Positive feedback also has value when it is specific. Naming a clear test or a useful simplification reinforces a practice the team wants repeated.

Reach a decision

Many review tools offer three outcomes: comment, approve, or request changes. A comment shares feedback without deciding. Approval says the change meets the applicable standard. Request changes identifies work required before approval.

Approval is not a claim of perfection. Judge whether the proposal improves the codebase and satisfies its goal. Optional polish can remain optional. Conversely, a passing test suite does not require approval when the design, behavior, or risk is unacceptable.

Technical facts, requirements, and agreed standards should control disagreements. When a written exchange stops producing progress, discuss the issue directly and record the conclusion in the review. Escalate through the team's ownership path when consensus remains out of reach.

Keep the loop healthy

Review is a conversation with response time, not a one-time inspection. Prompt feedback reduces waiting and lets the author revise while the context is fresh. If you cannot complete the review soon, communicate when you can respond or help find another qualified reviewer.

Small, focused changes shorten this loop. They reduce the amount of context a reviewer must hold and make defects easier to isolate. A focused change may still be difficult, but its difficulty comes from the problem instead of unrelated edits.

Review automation and human judgment as one system. Let tools handle deterministic checks. Use human attention for intent, design, risk, clarity, and tradeoffs. Neither part replaces the other.

Limits

Code review samples only the evidence available before merge. A reviewer can misunderstand the system, miss a path, or trust an incomplete test. Review does not replace testing, security analysis, observability, staged delivery, or incident learning.

Mandatory approval can enforce that a review occurred. It cannot guarantee that the review was careful or that the reviewer had the right expertise. Match reviewers and review depth to the change's risk and ownership boundaries.

Code review also has a social cost when comments become vague, personal, or dominated by preferences. Clear standards and explicit severity keep the discussion focused on the code and its consequences.

A path to stronger practice

First, learn to state the change's intent in one sentence. Then practice reading from purpose to design, tests, implementation, and details. Write comments that include evidence, consequence, and expected outcome.

Next, calibrate with your team. Compare which findings block approval, which remain suggestions, and which belong in automated checks. Study missed defects and review delays without turning the review into a blame exercise.

Finally, add specialist review where the risk demands it. Security, privacy, concurrency, data migration, and accessibility may require different expertise. A mature review process directs the right attention to the right change.

Relevant careers