openbranch
Reading as a guest. Sign in to save what you have read and practised, and pick up where you left off.
Learning path·Code Review

The life of a breaking change

A null enters a codebase hidden inside a 380-line PR. Follow it: catch it in review, work out who it breaks, pin it down with tests, and document it before it catches the next person.

Almost no breaking change arrives announced. It arrives inside a PR that fixes something real, written by someone acting in good faith, on a line that does not look important.

In atlas/gateway that line changed Promise<Response> to Promise<Response | null>. The timeout fix was correct: a request to a slow upstream no longer hangs forever. The return type came along for the ride.

This path follows that null from end to end. You catch it in a diff you would rather approve without reading, decide who it actually breaks, pin it down with tests so it cannot quietly disappear, and document it so the next person does not fall into the trap.

They are four different jobs on the same defect, and only the first looks like what people call "reviewing code". The other three are what decide whether that null is a decision or an accident nobody remembers making.

5 steps~67 min
Start with the guideStep 1 of 5 · Review culture
Start the path

Part 1 of 3

Catch it

PR #214 fixes a real timeout and carries plenty of noise: a forgotten console.log, cleanup that had nothing to do with it, and a changed return type. First why noisy PRs get rubber-stamped; then it is your turn to review it.

Step 1 of 5GuideAvailable

Review culture

Code review practices that don't stall PRs: size limits, Conventional Comments labels, and the calibration between request-changes and approve-with-comments.

7 min read
Open guide
Step 2 of 5ChallengeAvailable

Review a noisy pull request

A 380-line PR bundles a real fix with unrelated churn and a breaking contract change. Review like a maintainer: keep what belongs, flag what doesn't.

moderate20 min+20 pts

Part 2 of 3

Work out who it breaks

You flagged it. Now the uncomfortable question: breaking for whom? The answer is not the same for a library you publish as for two services you deploy together this afternoon.

Step 3 of 5GuideAvailable

SemVer in practice

When to bump major, minor, or patch — and why SemVer is the wrong tool for services. Includes a consumer-first changelog format that people actually read.

7 min read
Open guide

Part 3 of 3

Live with it

PR #214 was merged. The null is real and permanent now, and not one test or line of documentation backs it up. What remains are the two jobs nobody celebrates.

Step 4 of 5ChallengeAvailable

Write tests for fetchUpstream

The timeout fix landed but nothing proves it. Write the tests that lock in the new behavior before it can quietly regress.

moderate18 min+20 pts
Step 5 of 5ChallengeAvailable

Document the gateway module

fetchUpstream has no docs, no examples, and a non-obvious null return. Write the documentation that the next contributor needs before they can use it safely.

beginner15 min+10 pts