30 May 2024 / Technical leadership / 8 chapters

Put review before incompatible code exists

From How technical decisions break down as a team grows

If review happens late, a design question turns into a negotiation about work that's already done. By the time a big pull request reveals a changed contract, it might include migrations, tests and interface work that all assume the new behaviour, and reviewers are stuck between accepting it and asking for expensive rework.

I'd bring shared decisions into review once the shape of the change is known but the implementation options are still open. A short proposal on the work item is usually enough. It should name the existing decision, the change being asked for, the affected components, migration concerns and open questions.

Match the depth of review to what's at stake. A small compatible extension might only need one owner to confirm asynchronously. A change to a state model several systems use is worth a working session with the consumers. Anything touching permissions, financial records or a migration you can't undo needs proper evidence and sign-off from whoever's responsible for those consequences.

Reviewers should run the proposal against concrete cases:

  • What happens to existing records?
  • Can the old and new versions run side by side during deployment?
  • How does a consumer tell the change has happened?
  • What will support staff see?
  • What happens if one component deploys and another doesn't?

Those catch integration problems far earlier than a general feeling that the design looks fine.

Put a time limit on the review path and make its status visible. A proposal with nobody responsible for answering it encourages teams to just go ahead quietly. Say who has to respond, which concerns are blocking and when it'll be looked at again. For a change that matters, silence shouldn't count as approval.

Keep the code review tied to the accepted decision. The pull request should link the record and show how the tests cover the agreed behaviour, which is how you catch drift between the approved design and whatever turned out easiest to build.

Sometimes review confirms there's no shared decision involved, which is a useful answer, and the owning team can carry on without coming back for permission. I'd only record that if the boundary is likely to be questioned again.

To see whether this is working, check whether affected teams see a contract change while there's still room to alter it, and whether the resulting decision is there for the next person who works in that area. Counting approvals won't tell you either.

All articles