Architecture & design · Book notes
Looks Good to Me and the case for writing down review rules
Every team already has a code review process, and Looks Good to Me argues the real decision is whether to write it down.
You put up a big PR that touches most of your service. Someone on your team approves it a few minutes after you put it up with the comment “LGTM”. There is no way to read the diff in a few minutes, so you kind of doubt that they actually reviewed it. But you can’t really say that it wasn’t a review, because nobody documented what the purpose of a review is.
This is where Adrienne Braganza’s Looks Good to Me starts. She says that every team has a review process, whether it’s designed or not. The difference is that it’s not a process you chose, and you have nothing to point to when a defect slips through.
The real decision is what to do about the process you already have. Do you leave it unwritten so people keep guessing? Do you implement a lot of heavy policy (mandatory checklists and sign-offs)? Or do you write a short team working agreement that the team owns and can edit as it learns?
I think there are some people who kind of take a badge of honor in not having any sort of process. And a lot of times you just need process.
For teams who think they’re too fast for process: I’d argue that a working agreement is the lightweight option. The heavy option is unwritten process, where every reviewer has their own private set of rules and disputes start by arguing about whose rules apply. An agreement can delegate nitpicks to linters/formatters so you can spend review time on intent.
The unwritten cost is ambiguity. Who can block a merge? When is it okay to self-approve a PR (overnight outage)? Braganza says the agreement should anticipate emergencies and edge cases. She also describes it as a living document that evolves with the team.
Having repeatable processes that anyone can step in and do doesn’t mean that you’re replaceable. It just means that the organization has reduced their risk.
If you write down the rules, then the one person who knows them can actually go on vacation. Let’s say you have a rule that’s intentionally bad, like “in all disagreements, the developer always wins over the architect”. I would argue about that in the first meeting, probably at length, but at least the whole team would know where they stood.
I would argue that that’s dysfunctional, but at least it wouldn’t be ambiguous.
The open edge is that it’s only as good as the last time the team got around to revising it. If it’s a living document that doesn’t get edited, it’ll turn into an unwritten process plus a file, and you’ll go back to quickly LGTMing. What makes a team want to reopen it? Is it a bad incident that got through review, or is it a new hire asking why things are done this way?