Engineering Standards

Engineering Standards

Conventions exist to remove decisions that do not matter, so attention goes to the ones that do. The specific choice matters far less than the consistency — three ways to do the same thing in one codebase is a defect even when all three work.

Code that reads well

Optimise for the reader. Code is read many more times than it is written, and usually by someone who has forgotten writing it.

• Names carry the meaning. daysUntilExpiry beats d. A good name removes the need for a comment.
• Comments explain why, never what. The code says what. A comment that restates the line is noise that will go stale; one that records the reason ("the provider returns 200 on failure, so we check the body") is permanent value.
• Functions do one thing at one level of abstraction. Mixing "orchestrate the checkout" with "format a date string" in one function forces the reader to change altitude mid-sentence.
• Early returns over nesting. Handle the failure and leave; keep the happy path unindented.
• Make illegal states unrepresentable. A discriminated union beats a struct with six optional fields and a comment explaining which combinations are valid.
• Explicit over clever. The clever line saves you a minute now and costs someone twenty later.
• Consistent error handling. One approach per codebase, applied everywhere.

Match the codebase you are in. Even where you would have chosen differently. Argue for the change in a PR of its own; do not smuggle it in alongside a feature.

Git workflow

Trunk-based, with short-lived branches, is right for the large majority of teams. Long-lived branches accumulate merge risk in direct proportion to their age.

main ──●──●──●──●──●──●──►   always deployable, protected
        \        /
         ●──●──●             feature branch: 1-3 days, then merged

Use Git Flow only if you genuinely ship versioned releases to customers who choose when to upgrade. For anything continuously deployed it is friction with no benefit.

Branch protection on main: no direct pushes, review required, status checks required, force-push disabled.

Commits

feat(billing): add proration for mid-cycle plan changes

Customers upgrading mid-cycle were charged the full new price.
Now we credit the unused portion of the current period and charge
the difference, matching what the pricing page promises.

Fixes #482

• Imperative mood, present tense: "add", not "added" or "adds". It reads as an instruction to the codebase, which is what a commit is.
• Subject under ~72 characters. Body explains why, wrapping at 72.
• One logical change per commit. A commit that changes formatting and behaviour is a commit nobody can review or revert cleanly.
• Conventional Commits (feat, fix, chore, docs, refactor, test, perf) if you want automated changelogs and semantic versioning. Adopt it as a whole or not at all — half-applied it just adds noise.
• Never wip, fix, stuff, asdf, or . as a commit message on main.

Pull requests

Small. Review quality falls off a cliff past a few hundred lines. A 1,000-line PR gets "LGTM"; a 100-line PR gets read.

## What
One or two sentences.

## Why
The problem this solves. Link the issue.

## How
Only if the approach is non-obvious. Note anything you rejected and why.

## Testing
What you tested, and how a reviewer can verify it.

## Screenshots
For any UI change. Before and after.

## Checklist
- [ ] Tests added or updated
- [ ] Docs updated if behaviour changed
- [ ] No new personal data collected, or the privacy inventory is updated
- [ ] Migration is reversible and tested

The author's job is to make review easy. Separate refactors from behaviour changes. Leave comments on your own diff explaining anything surprising. Say what you are unsure about — that is where you want attention.

Code review

What review is for: correctness, missed edge cases, security, whether the change fits the system, and knowledge transfer. It is not for style — that is the formatter's job, automated, with no human discussion.

Reviewer conduct

• Review within one working day. A blocked PR is a blocked person, and the branch is getting staler.
• Distinguish blocking from optional. Prefix non-blocking comments — nit:, suggestion:, question:, praise:. Without this, every comment reads as a demand.
• Ask, do not assert. "What happens if this is null?" beats "this will crash" — and is right more often.
• Say what is good. Review that is only criticism trains people to dread it.
• Approve with comments when the remaining points are minor. Blocking a PR over a preference is a tax on the whole team.

Author conduct

• Respond to every comment, even if only to say "good catch, fixed".
• Disagree with reasons, not by ignoring. If you and the reviewer cannot converge in two rounds, talk — a thread with fifteen replies is a conversation that should have been a call.
• Do not force-push mid-review; it destroys the reviewer's ability to see what changed.

Escalate rather than block indefinitely. A PR stuck for a week is a process failure, not a code failure.

Documentation

Write the documents that are read. Skip the ones that are not.

The rule: documentation that is not generated, or not exercised, decays. Generate API docs from the code. Exercise runbooks in drills. Keep the README in CI by having the setup script be the thing the README describes.

ADRs are the highest-value-per-word document in this list. One page — context, options, decision, consequences — for every one-way door. Six months later it is what stops the team relitigating a settled question, and what tells a new engineer whether a constraint is a decision or an accident. See project-blueprint/templates/adr.md.

CLAUDE.md / AGENTS.md deserve a specific mention now that AI assistance is routine. A short file describing your conventions, your directory layout, your test commands and your non-negotiables makes AI-generated code match the codebase instead of fighting it. It costs twenty minutes and pays back on every subsequent change.

Onboarding

The test: hand a new engineer the README and time them. If they are not running the project in 15 minutes, the README is wrong — not the engineer.

The first week should produce a merged pull request. Not a large one — a real one. It exercises the whole path (setup, convention, review, CI, deploy) and it finds every broken step in the process.

Have the newest person fix the onboarding docs. They are the only one who can see what is missing; everyone else has forgotten.

Developer experience

DX is not comfort. It is the rate at which the team can safely change the system.

Every hour spent on the feedback loop is repaid many times over, by every engineer, on every change.

Technical debt

Debt is not a moral failure — taking it on deliberately to ship is often correct. The failure is taking it on invisibly.

Make it visible.

## Debt register

| # | What | Where | Cost now | Cost if unfixed | Effort | Trigger |
|---|---|---|---|---|---|---|
| 1 | Payment retry logic duplicated in 3 places | billing/ | Bugs fixed 3× | Divergence → double charges | M | Next billing change |
| 2 | No index on `events.user_id` | schema | 400ms on the timeline | Linear degradation | S | At 1M events |
| 3 | Auth logic in the controller | api/auth | Untestable | Blocks the SSO feature | L | Before SSO |

The "trigger" column is what makes this useful. Debt with no trigger is never repaid; debt with a trigger gets fixed at the moment it is cheapest, because you are already in that code.

The boy-scout rule, bounded: leave code better than you found it — within the scope of what you are doing. Do not bundle a refactor into a bug fix; the reviewer cannot then see the fix.

Do not rewrite. Recommending a rewrite is almost always wrong and always expensive. If you genuinely believe it, name the specific unfixable property that forces it, and price it honestly. Then consider strangling it incrementally instead.

The audit questions

1. Could a new engineer set this up in 15 minutes from the README alone?
2. Is there one way to do each thing, or several?
3. Are the expensive decisions recorded anywhere?
4. Does CI block merge, and does anyone bypass it?
5. What is the median PR size and review turnaround?
6. Is there a runbook for the three most likely failures, and has anyone run it?
7. Is known debt written down with triggers, or carried in one person's head?
8. What is the bus factor on deploys, on the database, and on the payment integration?

References

• references/conventions.md — code style, naming, structure, comments
• references/git-and-review.md — branching, commits, PR and review practice in depth
• references/documentation.md — what to write, what to generate, what to skip