Code review
Review is a design and risk-control practice, not a gatekeeping ritual. Be direct about code and generous about people.
Author checklist
- Keep the PR focused; split refactors from behavioral changes.
- Explain what changed, why now, how it was tested, and how to roll it back.
- Add screenshots, recordings, or map coordinates for visual changes.
- Self-review the diff before requesting review.
- Respond to every comment or resolve it with an explanation.
Reviewer checklist
- Confirm the change solves the linked issue and acceptance criteria.
- Check correctness, boundaries, failure behavior, security, and data migration risk.
- Inspect tests for meaningful assertions, not just execution.
- Check spatial assumptions: CRS, geometry validity, scale, licensing, and performance.
- Approve only work you would be comfortable supporting in production.
Comment labels
Use a prefix so authors can prioritize:
| Prefix | Meaning |
|---|---|
blocking: |
Must change before merge. Explain the risk. |
question: |
Need context before deciding. |
suggestion: |
Non-blocking improvement; author decides. |
nit: |
Minor style issue; do not block. |
praise: |
Name a good decision worth repeating. |
Approval standard
Approve when the implementation is correct, understandable, tested proportionately, and operationally safe. Request changes when the PR has a material correctness, security, maintainability, or delivery risk. Escalate disagreements to the tech lead quickly; do not let a PR stall.
flowchart TD
A[PR opened] --> B{Checks pass?}
B -- No --> C[Author fixes]
C --> B
B -- Yes --> D[Review]
D --> E{Material concern?}
E -- Yes --> C
E -- No --> F[Approve and merge]