Reviews and design docs

Why it matters

Two habits separate teams that ship reliably from teams that keep reworking the same features: they review code before it merges, and they write down designs before building anything large. Both are about catching problems while they are cheap. A mistake found in a one-page design doc costs an hour of discussion. The same mistake found after three weeks of code costs three weeks. Code review and design docs are also how knowledge spreads, so no single person is the only one who understands a part of the system.

Code review

A pull request (PR, called a merge request on GitLab) proposes a change and asks teammates to review it before it joins main.

As the author:

• Keep PRs small: a few hundred changed lines at most. Large PRs get skimmed, not reviewed.
• Write a description that says what changed, why, and how you tested it. Link the issue.
• Review your own diff first. You will catch leftover debug code and typos.
• Separate refactors from behaviour changes, ideally in separate PRs.

As the reviewer, look for:

1. Correctness: does it do what the description says, including edge cases and errors?
2. Tests: is the new behaviour tested, and would the tests fail if the code were wrong?
3. Design: is it in the right place, and is it simpler than it could be?
4. Security and data: input validation, secrets, permissions, personal data in logs.
5. Readability: names, comments that explain why, consistent style (leave pure formatting to a linter).

Write comments that are kind and specific. Comment on the code, not the person. Ask questions when you are unsure ("What happens here if items is empty?"). Mark optional suggestions clearly, for example with a nit: prefix, so the author knows what blocks the merge and what does not. Approve when the change makes the codebase better, even if it is not exactly how you would have written it.

Design docs

A design doc (also called a technical design document or RFC) describes a change before it is built, so others can review the approach. Not every change needs one. Write one when the work will take more than a few days, touches several components or teams, changes a data model or public API, or has a choice between options that are hard to reverse.

A short design doc template that works:

# Saved searches for job listings

**Author:** Sam Rivera · **Status:** In review · **Reviewers:** Priya, Jon

## Context
Users repeat the same search every day. Support gets ~20 requests a month for alerts.

## Goals
- Users can save up to 10 searches and re-run them in one click.
## Non-goals
- Email alerts (a later project).

## Proposal
New `saved_searches` table (user_id, name, query JSON, created_at).
Endpoints: GET/POST/DELETE /api/saved-searches. Limit 10 per user, enforced in the API.

## Alternatives considered
- Store in browser localStorage: no cross-device sync, so rejected.

## Risks and open questions
- Query JSON format may change when filters change: version it (`v: 1`).

## Rollout and testing
Behind a feature flag; unit tests for the limit; enable for staff, then 10%, then all.

The most valuable sections are usually non-goals (they stop scope from growing) and alternatives considered (they show you chose deliberately and let reviewers suggest a better option before code exists). Keep it short: one to three pages. A design doc is a tool for a conversation, not a contract. Share it, collect comments, update it, and record the final decision.

Many teams also keep architecture decision records (ADRs): one short file per significant decision ("We use PostgreSQL for job data because..."), stored in the repository so the reasoning survives after people move on.

Working together on the doc

• Share the doc early, while it is still easy to change your mind.
• Ask specific reviewers for specific input ("Priya, does the API shape fit the mobile app?").
• Resolve comments in the doc itself, or meet briefly when a thread goes back and forth more than twice.
• When the code is built, link the PRs from the doc, and note any places where the design changed.

Common mistakes

• Thousand-line PRs with a one-word description.
• Reviews that only nitpick style while missing a logic bug.
• Comments that sound like personal criticism.
• Design docs written after the code, just for the record.
• Design docs with no alternatives, which usually means only one option was considered.

Recap

• Small PRs with a clear what, why and how-tested; reviewers check correctness, tests, design, security and readability.
• Be kind and specific; label optional suggestions.
• Write a short design doc for big or hard-to-reverse changes: context, goals, non-goals, proposal, alternatives, risks, rollout.

Try it

In the Do tab, write a one-page design doc in a GitHub repository, open a pull request for it and run a review.