Every pull request to a protected branch needs at least one independent approval from someone other than the author. Branch protection enforces this, including for administrators.
Three narrow exceptions apply only to non-production repositories that are formally approved for them: trivial changes, incident emergencies, and pair programming. Note each exception in the pull request.
Review pace
Reviews should follow this pace:
| Pull request | Starts | Completes | Escalate |
|---|---|---|---|
| Standard feature | 24 hours | 48-72 hours | 24 hours, no response |
| Hotfix (P0/P1) | 2 hours | 4 hours | 2 hours, no response |
| Security-sensitive | 24 hours | 72 hours, with Security | 48 hours, no progress |
| Refactoring | 48 hours | 72 hours | 72 hours, no response |
The table sets three expectations. Authors are responsible for getting the change merged, finding a qualified reviewer within the hour, following up at 24 hours, and escalating to their EM at 48 hours. Reviewers should treat requests as interrupts, not background work. They should decline immediately if they cannot meet the time window. Engineering Managers are responsible for throughput, must plan review as real capacity, and must act within four hours of an escalation.
Keep pull requests as small as the change allows. Reviews are better when the reviewer can understand the whole diff. If work can be split into steps, stack the pull requests instead of landing one large change.
How enforcement is configured
Four-eyes is a control, not a convention. A protected branch must use the settings below. This lets a team verify that a repository complies instead of trusting people to follow the rule. Configure the settings as a GitHub ruleset (preferred) or classic branch protection.
| Setting | Value | What it prevents |
|---|---|---|
| Require a pull request before merging | on | Direct pushes to the protected branch |
| Required approving reviews | at least 1 | Merge without independent sign-off |
| Require review from Code Owners | on | Approval by an unqualified reviewer |
| Dismiss stale approvals on push | on | An approval that predates the merged diff |
| Require status checks to pass | on | Merge over a red build |
| Block force-pushes and deletions | on | Rewriting or removing protected history |
| Include administrators | on | A bypass path for privileged accounts |
| Require signed commits, linear history | on | An unattributable or tangled audit trail |
NT-NinjaTrader/cloud is the reference implementation. It has three active rulesets (master, the release branch, and sre/apphub-host-develop), each with enforcement set to active. When protecting a new repository, use the same required approving-review count and Code Owner requirement.
The equivalent ruleset excerpt:
{
"rules": [
{
"type": "pull_request",
"parameters": {
"required_approving_review_count": 1,
"require_code_owner_review": true,
"dismiss_stale_reviews_on_push": true
}
},
{ "type": "required_linear_history" },
{ "type": "required_signatures" },
{ "type": "non_fast_forward" }
]
}See GitHub’s protected branches, rulesets, and code owners references. CODEOWNERS identifies the qualified reviewer. See code ownership. Independent approval separates the author from the approver. Audit responsibilities records this as evidence.
What a review must verify
Approval means that a reviewer checked the change, not just that they saw it. A review confirms:
- Correctness. The change does what the pull request says, and edge cases are handled.
- Tests. They exist, are meaningful, and cover the new behaviour rather than restating it.
- Security and authz. The change does not widen access, leak secrets, or trust unvalidated input. See security.
- Backward compatibility. API, schema, and message contracts stay compatible, or the pull request notes the break.
- Observability. New paths emit the logs, metrics, and traces needed to operate them. See observability.
Self-approval and stale approvals
An author must never approve or merge their own pull request. The approver must be a qualified reviewer listed by CODEOWNERS, not just any available teammate.
Dismiss-stale-approvals ties each approval to the exact diff that will merge. If new commits are added after an approval, the approval is cleared and the change must be reviewed again. This prevents the approve-then-push-more pattern.
Automated and revert pull requests
Dependabot, Renovate, and automated reverts still need one independent human approval and all required status checks. Automation makes the merge process shorter; it does not remove the second pair of eyes.
Auto-merge is safe because it waits for these gates. It enables the merge only after approvals and checks are satisfied:
# .github/workflows/auto-merge.yml
on: pull_request
permissions:
contents: write
pull-requests: write
jobs:
enable:
if: github.actor == 'dependabot[bot]'
runs-on: ubuntu-latest
steps:
- name: Enable auto-merge
run: gh pr merge --auto --squash "$PR_URL"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
PR_URL: ${{ github.event.pull_request.html_url }}This uses the pull_request trigger, not pull_request_target. pull_request_target is privileged, runs against the base repository, and could let an untrusted fork branch escalate. Enabling auto-merge only starts the merge process behind the branch-protection gates. The required human approval and checks still apply.
Every P0/P1 hotfix pull request must link its incident ticket. The CODEOWNERS approver requirement stays the same during the expedited window. See release and incidents. The window makes the review faster, but never removes the approval.