Course outline · 0% complete

0/28 lessons0%

Course overview →

Code review basics

lesson 8-2 · ~10 min · 22/28

What review is for

A code review is a teammate reading your pull request before it merges. It is not a trial, and it is not reserved for beginners. Teams review everything, including the tech lead's work, because it reliably catches what authors cannot see in their own changes.

  • Bugs and edge cases. What happens here when the list is empty?
  • Clarity. If the reviewer cannot follow it, neither can the next maintainer, who might be you in a year.
  • Consistency. Naming, structure, and conventions matching the rest of the codebase.
  • Knowledge spread. After review, two people understand this change instead of one.

Reviewers comment on specific lines of the PR's diff, which is exactly the diff reading you practiced in lesson 3-2, so the skill transfers directly.

Comments vary in weight, and reading that weight correctly is part of the skill. Some are blocking, as in this breaks empty carts, and some are take-it-or-leave-it preferences, often marked with a leading nit: so you know the author of the comment does not intend to hold anything up over it.

The robots review first: CI

On real teams, opening or updating a pull request automatically triggers CI, continuous integration. A server checks out your branch and runs the project's automated checks, which typically means the test suite and style checkers, and sometimes a full build.

The PR page shows the result as a green check or a red ✗, and most teams configure GitHub to refuse the merge button while checks are red.

The division of labor has a clear logic behind it. Reviewer attention is the scarcest resource on a team, so machines take the mechanical problems, broken tests and formatting, and humans spend their limited attention on what machines cannot judge, meaning design, naming, and edge cases.

In practice that means a red ✗ is your signal to fix and push again before asking anyone to look. Requesting review on a red PR wastes a teammate's pass through the code, since half their comments will be about failures you already had the information to fix.

Getting through review smoothly

Responding to feedback. Make the fixes as new commits on the same branch and push. The PR updates automatically, there is no resubmit step, and the conversation simply continues on the new code. Reply to each comment, and when you disagree, say why, because reviews are discussions rather than orders.

The single biggest kindness is keeping PRs small. A 100-line PR gets a careful review in minutes. A 3,000-line PR gets a tired skim and a week of delay, which is worse for everyone including the author. Small, focused branches, one bug fix or one feature slice each, are the team-scale payoff of the small-commits habit from lesson 2-3.

Write the description for the reviewer. What changed, why, and how you tested it. You are saving them the archaeology, and the reviewer who does not have to reconstruct your reasoning has attention left over for the code itself.

To get your fixes into an open pull request, commit them on the same branch and push. The PR picks them up automatically.

A PR tracks a branch rather than a frozen snapshot, so any commit pushed to that branch appears in the PR immediately and the review continues from there.

There is deliberately no resubmit button, which surprises people coming from ticket-based tools. The PR is a live view of the branch, so the workflow for addressing feedback is the same workflow you already use for writing code.

Experienced engineers keep pull requests small because small PRs get faster, more careful reviews and are far easier to revert if something turns out to be wrong.

Reviewer attention is the scarce resource. A focused PR gets genuine scrutiny and merges quickly, while a huge one gets skimmed, which is the worst of both outcomes: slow and shallow.

The second half matters after the merge. If a small PR proves to be a mistake, git revert from lesson 4-3 cleanly cancels one contained change. Reverting a giant tangled PR means untangling which parts were the problem, often while something is broken in production.

A red ✗ from CI means the automated checks failed on your branch, and the professional next move is to fix the problem, push new commits, and let CI go green before requesting review.

CI runs the project's checks against your branch on every push, so the ✗ is reporting a real failure in tests, linting, or the build. Most teams block the merge button while it is red, so nothing can proceed until it is dealt with regardless.

Pushing a fix re-runs the checks automatically, which makes the loop short. Going green before asking for review respects your reviewers' time, since a human should not be spending their pass through your code on problems a machine already identified.