Review what a linter cannot

---name: code-reviewdescription: "Review a change for what a linter cannot see: whether it is correct, what it breaks elsewhere, and which parallel list it forgot to update."metadata:  title: "Review what a linter cannot"  category: engineering  tags: [review, correctness]--- ## When to use this A change needs judgment rather than checking. Formatting, import order andnaming are already a tool's job; spend nothing on them. ## How 1. Read enough of the code around the change to know what it is for. A diff   reviewed on its own can only be judged on style.2. Ask what ELSE has to know about this. Most defects that reach a user are a   hand-maintained thing that mirrors a type and did not get the new member: a   patch object, a switch, a mapper, a column list in an SQL statement. A   compiler accepts a subset without complaint.3. Walk the failure paths in this order -- what happens on error, what happens   when it runs twice, what happens when it is interrupted halfway through. A   finding is concrete inputs leading to a wrong result; anything vaguer is a   worry, and worries do not belong in a review.4. Check that each test proves the behaviour and not the shape of it. A test   asserting the flags a timeout sets will pass against a timeout that stops   nothing. Assert the claim the feature makes.5. Quote the lines under discussion, and say plainly when a change is fine. A   review that always finds something teaches people to skim reviews.