Before deploymentgit-branching-strategy

Pull Request Review Checklist

16 items ·8 critical ·7 warn ·1 info

How to use this checklist

This is what the human reviewer is expected to confirm before approving a pull request that touches infrastructure. It is deliberately split from the Git Repository Production Readiness checklist, because that one is about the repository as a whole and this one is about an individual change.

Use it as the rubric for the review comment, not as a separate form. The reviewer should be able to point at each item they checked, and the PR author should be able to see which items were considered.

Sign-off

Every critical item must pass. A failing critical item blocks the merge; it is not a note for later. Record the date, the reviewer, and the disposition of every item that did not pass.

Critical8 items

The PR changes one logical thing (one Terraform module, one Ansible role, one application, one policy set). Mixed-concern PRs that touch unrelated code paths are split or rejected at review.
The PR description states why this change exists, what it changes, what it does not change, the risk class, the rollback path, and links to a ticket or runbook. A description that reads "fix stuff" or "WIP" is rejected.
A CODEOWNERS-required reviewer has approved every changed path. The reviewer is not the author, and approval did not come from a self-approval bypass.
Production-bound changes have a second reviewer who is independent of the change author and of the first reviewer. The second reviewer is named in the PR conversation, not implied by a CODEOWNERS rule.
All required status checks have passed on the latest commit, and no check has been skipped or dismissed. Skipped checks (`continue-on-error`, allow-list) are not used to bypass required gates.
For Terraform and Ansible changes, the CI-produced plan or diff artifact is linked in the PR conversation, and the human reviewer has read the diff in the PR, not just the summary output.
A secret scan (gitleaks, trufflehog, or platform-native) ran on the PR and the result is green. The reviewer has not hand-waved the secret scan by dismissing the check.
The PR does not copy a real-looking secret from anywhere — not even as a "placeholder" that looks like a credential. All test data is obviously synthetic (`example`, `changeme`, AWS-key-shaped strings have been replaced).

Warning7 items

The diff is within the team's review budget: under ~400 changed lines, or explicitly waived with a documented reason in the PR description. A diff larger than 1000 lines has been decomposed or annotated with walkthroughs.
```
git diff --shortstat origin/main...HEAD
```
The PR contains no debug prints (`console.log`, `print()`, `kubectl debug`, `terraform -no-color` traces in CI logs) that should not have been committed. WIP commits have been squashed or removed.
After push, the branch shows a linear history of intended commits. A PR with a rewritten history that obscures the reviewed diff (force-push mid-review) is sent back for re-review.
Breaking changes (renamed outputs, removed variables, schema changes, dropped Ansible variables, removed Kubernetes fields) are called out in the PR description with a migration note and a deprecation timeline.
Behaviour-changing code (modules, roles, controllers) is accompanied by tests, and the tests have actually run in CI rather than being skipped or marked xfail.
Changes that require out-of-band action (manual state import, manual key rotation, manual data migration, runbook update) are flagged with a checklist in the PR description, not left for the operator to discover during deploy.
New dependencies (actions, modules, collections, providers, container images) are pinned by full digest, and the source (registry, marketplace, GitHub org) is acceptable to the security owner.

Info1 item

The merge method is consistent with the team policy (squash, rebase, or merge commit) and is configured in the repository, not chosen per-PR. Production-bound branches do not allow merge commits that complicate revert.