top of page

Tutorial 7: Build Review Conventions

  • Contributor
  • 4 days ago
  • 2 min read

Conventions cut review time. Less debate; more action.

Step 1: Document Your Review Process (10 min)

A short doc:

# Code Review Process

## Authors
- PRs under 300 lines
- Filled-out description
- Self-reviewed before requesting review
- Linked to a ticket

## Reviewers
- Acknowledge within 1 business day
- Approve, request changes, or comment
- Prioritize critical-path code
- Use suggestion blocks where possible

## Approvals required
- 1 approval for non-prod
- 2 approvals for prod / migrations
- Mandatory reviewer (CODEOWNERS) for security / infra

## Merge
- Squash and merge (default)
- Author merges after approval

Pin to your wiki. Onboard new hires.

Step 2: PR Templates (10 min)

.github/pull_request_template.md:

## Why
[Motivation; problem]

## What
[Change summary]

## How to test
[Steps]

## Risk
[Could this break?]

## Related
[Tickets]

Authors fill in. Reviewers see context. Consistency.

Step 3: Comment Tags (10 min)

Conventional Comments:

nit: trailing whitespace
suggestion: use Map instead of object
question: why this branch?
issue: this will fail on empty input
praise: clean approach!
thought: maybe extract to a helper?

Author knows: nit = ignorable. Issue = must fix. Question = needs answer.

Adopt across the team. Less ambiguity.

Step 4: CODEOWNERS (10 min)

.github/CODEOWNERS:

# Default
* @engineering-team

# Auth code requires security team
/src/auth/* @security-team

# Database migrations require senior eng
/migrations/* @senior-eng

# Docs anyone
/docs/* @engineering-team @docs-team

Right reviewers auto-requested. Prevents "I didn't know this was sensitive."

Step 5: Required Status Checks (10 min)

In GitHub: Settings → Branches → Require:

  • CI tests pass

  • Lint pass

  • Type check pass

  • Coverage threshold

  • Security scan

Merging blocked until checks pass. No "skip the failing test."

Step 6: Automate the Obvious (10 min)

  • Prettier / Black / gofmt — formatting

  • ESLint / Ruff / golangci-lint — linting

  • Conventional Commits checker — commit message format

  • Branch naming convention — feat/, fix/, chore/

  • PR title format — auto-fail if not conventional

Tools enforce. Humans review what tools can't.

Step 7: Define "Done" (5 min)

PR is done when:
- [ ] Tests cover the new code
- [ ] CI green
- [ ] At least 1 approval
- [ ] All comments addressed (resolved or rebutted)
- [ ] Documentation updated if user-facing

Explicit. No "is this ready to merge?" guessing.

Step 8: SLA on Review (10 min)

First response: 1 business day (acknowledgment; not full review)
Full review: 2 business days
After updates: 1 business day for re-review

Public expectations. Authors know when to nudge.

Some teams use bots (@reviewbot remind me) to track aged PRs.

Step 9: Periodic Review of Conventions (5 min)

Quarterly:

  • Are conventions still useful?

  • What's friction?

  • New patterns to standardize?

Conventions evolve. Don't fossilize them.

Step 10: Lead by Example (10 min)

The team lead's PRs set the bar:

  • Small PRs

  • Good descriptions

  • Quick review responses

  • Tone in comments

Conventions documented and not modeled = ignored.

Conventions modeled by leaders = adopted.

What You Just Did

Review conventions: process doc, templates, comment tags, CODEOWNERS, status checks, automation, definition of done, SLA, periodic review, leadership modeling. Smoother reviews.

Common Failure Modes

Conventions documented; not enforced. Aspirational.

Too many rules. Burden; ignored.

No CODEOWNERS. Wrong reviewers requested; delays.

No PR template. Authors guess; reviewers re-request context.

Leaders skip the conventions. Team learns conventions don't matter.

bottom of page