Form Factory’s code review standards define how PRs are requested, reviewed, and approved. The process emphasizes author ownership, the Conventional Comments format, and a clear distinction between blocking and non-blocking feedback. For PR creation and requirements, see pull-request-guidelines.
Requesting Reviews
PR authors request reviews from a project-specific code owners team (group). If a reviewer plans to review within 4 hours, they should replace the team with themselves to clear the queue; otherwise, the team stays assigned so others can pick it up.
Author Responsibility
The PR author owns the review timeline. When reviews stall, they must:
- Follow up directly via Slack or in-person
- Escalate to team leads if blockers persist
- Flag blocked PRs in daily syncs
- Reassign reviewers if needed
- Keep the PR rebased to avoid merge-conflict delays
Performing Reviews
Reviewers use the Conventional Comments format. Approval rules:
| Situation | Action |
|---|---|
| All blocking comments addressed | Approve |
| Any blocking comment unresolved | Request changes |
| PR requirements not met | Request changes without full review |
A single code-owner approval is sufficient; additional reviews are welcome but not required.
Responding to Comments
Authors must respond to every blocking comment — either describing the action taken or explaining the reasoning. Resolution ownership:
- Blocking — only the commenting reviewer resolves
- Non-blocking — either party resolves
After addressing all blocking comments, the author re-requests review.
Blocking vs Non-Blocking Comments
Comments are blocking by default. A comment must be explicitly labeled non-blocking to be treated as such.
Non-Blocking (low risk / easy follow-up)
- Incomplete work tracked via follow-up tickets (see placeholder-development)
- Style, naming, or minor performance suggestions
- Future stories or backend wiring planned for later
Blocking (hard to track / could cause defects)
- Framework/API misuse — hooks, loader/action logic, promise handling
- Missing error handling — unhandled states, silent failures
- Weak types — unjustified
any/unknown, unsafeasassertions - Suppressed linting —
eslint-disableorts-ignorewithout explanation - Readability — unclear naming, large/duplicated functions, undocumented complex logic
- Convention violations — file placement, naming patterns, architecture, design system