Skip to content

PR reviews

How we propose, review, and merge changes. This page is the canonical version of our code-review guidelines (the old root CODE_REVIEW.md now points here).

Branch and PR naming

Every change maps to a PPL Jira ticket (see Ticket lifecycle).

  • Branch: PPL-NNNN/short-description — lowercase, hyphenated.
  • PR title / commits: type(PPL-NNNN): summary — Conventional Commits. commitlint enforces the type (feat, fix, chore, docs, refactor, test, …); the PPL-NNNN scope is our convention, not enforced (commitlint allows any scope), so add it consistently.

Do not rename a branch after the PR is open

Renaming after opening breaks the PR's head ref and the Jira ↔ GitHub link. Get the name right before you push and open.

How reviews work

  • PRs target develop (see Branching & releases).
  • Merge is squash into develop; the squash title keeps the type(PPL-NNNN): summary format.
  • CI must be green before merge — see GitHub workflows.

CONFIRM — approval rules

State the hard rules: how many approvals are required, whether CODEOWNERS approval is mandatory for owned areas, and which status checks are required branch-protection rules on develop and main.

For authors

  • Keep PRs small — focused, reviewable changes.
  • Write clear descriptions — what, why, and how. Link the PPL ticket.
  • Self-review first before requesting review.
  • Update the docs in the same PR when you change a documented behaviour or process.
  • Respond to feedback promptly; be open to suggestions.

For reviewers

  • Be constructive — focus on the code, not the person.
  • Be timely — aim to review within 24 hours.
  • Prioritise — separate blocking issues from nits.
  • Pull and test complex changes locally.
  • Treat a missing doc update like a missing test for process/behaviour changes.

Reviewer checklist

Functionality

  • [ ] Does the code do what it's supposed to, including edge cases and error conditions?
  • [ ] Is the logic correct and efficient, with no obvious performance issues?

Code quality

  • [ ] Readable, clearly named, well organised, no needless duplication (DRY)?
  • [ ] Follows the style guide and conventions in AGENTS.md?
  • [ ] Comments only where they explain a non-obvious why (no narration comments).

Testing

  • [ ] Appropriate unit tests, covering edge and error cases?
  • [ ] Integration / widget / E2E tests where needed, and all passing?
  • [ ] Adequate coverage.

Security

  • [ ] No vulnerabilities; inputs validated and sanitised.
  • [ ] Sensitive data, secrets, and API keys handled correctly.
  • [ ] Auth and authorization correct.

Documentation

  • [ ] Public APIs documented; breaking changes called out.
  • [ ] Relevant docs updated (this site, README, architecture docs).

Frontend (if applicable)

  • [ ] Accessible (WCAG), responsive, with loading and error states handled.
  • [ ] Null safety: prefer ?. over ! in build; guard async gaps with if (!mounted) return;.

Backend (if applicable)

  • [ ] Endpoints well designed and documented; rate limiting where needed.
  • [ ] Queries optimised; logging in place for debugging.
  • [ ] Validation via Zod schemas.

Dependencies & config

  • [ ] New dependencies necessary and version-pinned appropriately.
  • [ ] Config and environment variables documented.

Getting help

Questions about the review process or standards go in #tech.