Commits, PRs & Code Review

History is documentation. Six months from now, git blame is how someone finds out why a line exists, and the commit message is the only answer they'll get. Reviews keep that history good, but a slow review process costs more than the bugs it catches. The goal is both: a log that explains itself, and reviews that keep pace with the work.

Conventional Commits#

Commit messages, and so PR titles since PRs are squash merged, follow Conventional Commits 1.0.0:

<type>(<optional scope>): <description>

<optional body>

<optional footer(s)>

The format is a small tax that pays three ways: a reader scanning the log can skip every docs and chore line to find the behavior change that broke something, release tooling derives the version and changelog from the types, and a ! makes a breaking change impossible to miss in review.

Type	Use for
feat	A new capability for users
fix	A bug fix
docs	Documentation only
refactor	Code change that neither fixes a bug nor adds a feature
perf	A performance improvement
test	Adding or fixing tests
build, ci	Build system, dependencies, pipelines
chore	Everything else that doesn't touch behavior

feat(api): Api-Version header versioning per the versioning standard

fix(webhooks): verify signature before parsing the body

feat(payments)!: amounts are integer minor units

BREAKING CHANGE: `amount` is now an integer, see /docs/money/amounts.

• Imperative, lowercase, no period. "add", not "added" or "Adds": what the commit does when applied.
• The body says why. The diff already says what. "Retry on 503 because the processor sheds load during settlement" is worth more than any amount of "update retry logic".
• ! or a BREAKING CHANGE: footer for anything that breaks consumers. It maps straight onto a major version bump under SemVer, and onto a new version under Versioning & Deprecation.

Pull Requests#

• Small. Aim for under about 400 changed lines. Past that, reviewers stop reading and start skimming, and approval becomes a formality. Big changes ship as a stack of small PRs on a short-lived branch, or land dark behind a feature flag so each step is reviewable and deployable on its own.
• One concern per PR. A refactor and a behavior change in one diff means nobody can tell which lines change behavior, so neither gets a real review. Land the refactor first, then the change on top of it.
• Squash merge. One PR becomes one commit on main, with the PR title as its message, so git bisect lands on a change reviewed as a unit rather than a "fix typo" halfway through someone's branch. Small PRs make the lost intermediate commits cheap.
• Green before review. A reviewer's attention is the scarcest thing in the process; spending it on something CI would have flagged wastes it twice.

Description Template

The PR template asks four questions, because a reviewer who knows why the change exists and how it was checked reviews twice as fast:

## Why
The problem, and a link to the issue or design doc.

## What
The approach, and anything surprising in the diff.

## How it was verified
Tests added, manual steps, screenshots for UI.

## Risk and rollout
Migrations, flags, how to roll back.

Link the design doc for anything significant, and the ADR if the change departs from the playbook. The PR explains the change; the doc explains the decision.

Reviewing#

• First response within one business day. A PR waiting on review is work that can't ship, and its author has moved on and must page it back in. Most of the delay in review is waiting, not reviewing. One business day is the maximum Google uses, for the first response, not final approval.
• Design first, then correctness, tests and readability. If the approach is wrong, line comments are wasted effort. Style is the linter's job; a human arguing about formatting is a missing lint rule.
• Approve when it improves the codebase, not when it's how you'd have written it. Holding out for perfection teaches authors to batch changes, which makes reviews worse. Approving with small comments the author will address is fine.
• Ask for a split rather than reviewing a 2,000-line PR badly. A rubber-stamped big PR is worse than no review, because everyone believes it was reviewed.

Conventional Comments

Unlabeled comments leave the author guessing which ones block the merge, so they treat all of them as blocking and the review drags. Label them with Conventional Comments:

issue (blocking): this retries a POST without an idempotency key.
See /docs/architecture/retries-and-timeouts.

suggestion (non-blocking): extract the backoff calculation so the
webhook worker can reuse it.

nitpick: `amt` → `amount`, per our naming conventions.

praise: the migration plan in the description is excellent.

Label	Meaning
praise	Something done well. Use it; reviews that only find fault teach people to dread them
issue	A specific problem, ideally with a suggested fix
suggestion	A proposed improvement, with the why
question	A possible concern that needs an answer
nitpick	Trivial and preference-based. Never blocking
thought	An idea worth considering. Never blocking

The (blocking) and (non-blocking) decorations remove the remaining guesswork. Anything unlabeled is non-blocking.

References#

Specifications

• Conventional Commits 1.0.0, commit message format
• SemVer 2.0.0, how commit types map to version bumps

Reference

• GitHub: Configuring Commit Squashing, squash merges
• GitHub: Pull Request Templates, the PR description template

In practice

• Google Engineering Practices: Speed of Code Reviews, the one-business-day turnaround
• Google Engineering Practices: Small CLs, why PRs stay small
• Google Engineering Practices: The Standard of Code Review, approving changes that improve the code
• Conventional Comments, labeled review comments