Part 7 · 2 chapters · ~20 min

Code Review and Quality Culture

OWNERS as accountability made mechanical, readability as a per-language certification, a written review bar, small and stacked changes, lint as the repeated comments made executable, the culture the mechanisms protect, and quality as measured signals: review, tests, lint and types, dependencies, ownership and age, incidents.

15

OWNERS, readability and the review bar

who must approve, who may approve, what for
  1. OWNERS: a file per directory, inheriting up, enforced by the review tool (an approval from an owner of every changed file; cross-cutting changes under a policy). Owners are accountable: paged for the surface (part 8), asked by codemod bots (part 4), deciding the roadmap. Unowned code is a bug in the file.
  2. Readability: a per-language certification earned by having one's own code reviewed to the company's style and idioms until it passes consistently; holders may approve others' code in that language. It is how a thousand reviewers apply one style without a thousand copies of the guide, and a staff engineer new to TypeScript earns it like anyone.
  3. The bar, written down: approve if the change improves the overall health of the codebase, even if it is not perfect. Review design, correctness, complexity, tests, naming; prefix nits; never discuss what the formatter decides; reply within a business day; explain the why. A written bar can be appealed against.
  4. Small changes are the highest-leverage practice in this course and need no machinery: two hundred lines reviewed well in ten minutes versus two thousand reviewed badly in an hour. Stacked changes let large work land as many small reviews; review latency and change size are both dashboards.
  5. Lint as policy: anything a reviewer says every time becomes a rule with its reason in the message; the formatter ends style debate; readability covers what lint cannot see (design, naming, the right abstraction). The Architecture course part 0's tooling-enforced boundaries, applied to review.
  6. The culture the mechanisms protect: a conversation about the code, never the author; escalation to the design document and then a lead, with a clock; "LGTM with nits" as the normal outcome; two days unreviewed as a process bug; the bar as how new engineers learn the codebase.
OWNERS, READABILITY AND THE REVIEW BAR
who must approve, who may approve, and what a review is for
swipe the figure sideways, or tap expand for full screen
1/6
OWNERS
OWNERS: a file per directory listing users or groups, inheriting up the tree; the review tool requires an approval from an owner of every changed file; a change across many directories needs many owners or a global owner under a policy (part 1). Owners are accountable: they are paged for the surface (part 8), they decide its roadmap, and they are the ones a codemod bot asks. Unowned code is a bug in the file, fixed by assigning.
16

Quality as measured signals

the codebase's health, as a dashboard
  1. Review: time to first review (p50 under four hours, p90 under a day), time to land, the share of changes over 500 lines, reviewer load (two people reviewing 40% will burn out), changes landed with unresolved comments. Climbing latency predicts falling velocity next quarter.
  2. Tests: flake rate per test and suite against the budget (the Architecture course part 2), suite duration trend, quarantine count with age (past its date is a broken promise), coverage of critical flows (targeted) versus a percentage (reported).
  3. Lint and types: violations by rule (a rule nobody follows is fixed or deleted), suppressions with their trend (debt with a comment, and the comment is enforced), strict-mode adoption per package as a migration, the count of any.
  4. Dependencies: versions behind, known vulnerabilities by severity and age (a critical CVE open past N days pages someone), duplicate versions in the graph (the Architecture course part 5), licences, unused packages; a bot opens updates and the signal is how long they wait.
  5. Ownership and age: unowned directories and departed owners, code untouched for two years with no tests, bus factor per directory. Code search and history produce them; the platform reviews them quarterly.
  6. Incidents: by cause category, surface and detection source (alert, user, employee: the monitoring's report card), time-to-detect and time-to-mitigate trends, follow-up completion rate (a review whose actions nobody did is theatre). The SRE course owns the method; part 8 is the frontend on-call.
the transferable part
A team of thirty can put review latency, change size, flake rate, suppression count and CVE age on one page this week. The numbers that move are the culture; the dashboard is how a lead sees it before it is a problem.
QUALITY AS MEASURED SIGNALS
what a large organisation watches to know whether its code is getting better or worse
swipe the figure sideways, or tap expand for full screen
1/6
review
Review signals: time to first review (p50 under four hours; p90 under a day), time to land, change size distribution (the share of changes over 500 lines), reviewer load (who is reviewing everything and will burn out), and the rate of changes landed with unresolved comments. A team whose review latency is climbing is a team whose velocity will fall next quarter.