Design review

Hold code to the owner's design rules, and state what is wrong and what it should be instead. Findings are stated as findings, never softened into suggestions.

Pragmatism comes first. The rules serve maintainable software; they are not ends in themselves. Build what the problem in front of you asks for, and let an abstraction arrive with its second or third real case, never from principle. A rule applied where it adds structure without solving a problem is misapplied, and a finding that would do that is not raised. Pragmatism is not a shortcut, though: what is built is built properly, with no debt.

Files or a diff get the review below. A whole module, meaning its purpose, its history and a redesign, gets the module review, which runs this review as one of its steps.

The rules

• The patterns: Architecture and Code, for all code.
• The shared principles in principles, which a finding cites by name: fix the cause, no transitional architecture, no drift, Chesterton's Fence, no comments and rules as checks.
• Rules for particular kinds of code, loaded only when the scope has that kind:
  • UI, when the scope renders an interface;
  • API, when it defines an interface others call, such as a public module, a CLI command, a tool, an endpoint or a schema;
  • Agent, when it drives a model.

A diff review checks the Code rules, plus every Architecture rule the diff touches. A module review and a design document check both. Rules about stores, records, transactions and contexts apply only where the scope persists data or crosses a service boundary; a library or a UI package is not judged by them.