# Pull Request Template

> Copy the first half into `.github/pull_request_template.md` so it appears automatically. The
> reviewer checklist below belongs in your contributing guide.
>
> The purpose of a PR description is to let a reviewer answer "is this correct and safe" without
> reconstructing the author's reasoning from the diff.

---

## What this changes

**In one sentence:** _______________

**Why now:** _______________

**Issue / ticket:** _______________

## How it works

Two or three sentences on the approach, aimed at someone who has not been in this code for a
month. Explain the decision that was not obvious.

_______________

**Approaches considered and rejected:** _______________

## Risk

| | |
|---|---|
| Blast radius if this is wrong | |
| Reversible by rollback alone | yes / no |
| Database migration included | yes / no — backward compatible? |
| Configuration or secret change required | |
| Feature flag | name: ______ default: ______ |
| Performance impact considered | |
| Security impact considered | |

**If this breaks in production, what is the first symptom?** _______________

That question is worth more than most of the checklist. An author who cannot answer it has not
thought about how the change fails, only about how it works.

## Testing

- [ ] Tests added or updated, and they fail without this change
- [ ] Edge cases covered: empty, null, boundary, duplicate, concurrent
- [ ] Error paths tested, not only the success path
- [ ] Tested manually — how: _______________
- [ ] No new flaky behaviour introduced

**What is deliberately not covered by tests, and why:** _______________

## Reviewer notes

**Start here:** _______________ (name the file or function that carries the change)

**Anything you want a specific opinion on:** _______________

---

## Reviewer Checklist

> Review the change, not the person. Distinguish between defects and preferences, and say which
> you are raising — the most common cause of slow, resented reviews is unlabelled taste presented
> with the same weight as a bug.

## Correctness

- [ ] It does what the description says
- [ ] Edge cases: empty input, nulls, boundaries, duplicates, very large values
- [ ] Error handling: failures are handled or deliberately propagated, not swallowed
- [ ] Concurrency: shared state, race conditions, retries that could double-apply
- [ ] The failure mode is safe — when this goes wrong, does it fail closed?

## Safety

- [ ] Input validated at the boundary
- [ ] No secrets, tokens or keys in the diff
- [ ] Authorisation checked, not just authentication
- [ ] Personal data handled consistently with policy
- [ ] Logging does not include sensitive values
- [ ] Dependencies added are necessary and maintained

## Fit

- [ ] Consistent with how the surrounding code already works
- [ ] Not a duplicate of something that exists
- [ ] Naming says what the thing is
- [ ] Comments explain *why*, where the reason is not obvious from the code

## Operability

- [ ] It can be observed in production — logs or metrics for the new path
- [ ] It can be rolled back
- [ ] Migration is backward compatible with the running version

## Review conduct

| Practice | Why |
|---|---|
| Label each comment: **defect**, **question**, or **preference** | The author knows what blocks merge |
| Ask rather than instruct where you are unsure | You are often missing context |
| Approve with minor comments where trust allows | Blocking on preferences slows the whole team |
| Review within a working day | A PR waiting three days is worse than one imperfect merge |
| Comment on something done well | Reviews that only ever criticise are avoided |

🔴 If a review takes more than about an hour, the change is too large to review properly. Say so
and ask for it to be split — approving something you did not really read is worse than delaying
it.

## Sign-off

| | Name | Date |
|---|---|---|
| Author | | |
| Reviewer | | |
| Second reviewer (if high risk) | | |