Skip to content

feat: add PR approval requirements policy - #1734

Open
abdulraheemabdullahi30-tech wants to merge 1 commit into
rinafcode:mainfrom
abdulraheemabdullahi30-tech:governance/approval-requirements-policy
Open

abdulraheemabdullahi30-tech wants to merge 1 commit into
rinafcode:mainfrom
abdulraheemabdullahi30-tech:governance/approval-requirements-policy

Conversation

@abdulraheemabdullahi30-tech

Copy link
Copy Markdown

Overview

Adds Governance/policies/APPROVAL_REQUIREMENTS.md, the versioned reference for how many approvals a pull request needs, which automated checks must be green, and who may override a requirement.

The rules existed but were scattered: CONTRIBUTING.md §4 states branch-protection settings, §9 states the review policy, and §11 states the merge preconditions — and none of them answer "a migration that also touches a route: how many approvals, and from whom?". This document answers that as a table, makes the current gate status of each check explicit, and writes down the override rules that were previously implicit.

Related Issue

Issue #1600 — "Add PR approval requirements for TeachLink Backend".

Changes

New governance document

  • [ADD] Governance/policies/APPROVAL_REQUIREMENTS.md
    • §2 Minimum approvals per change type — a twelve-row table covering documentation, governance, tests, dependencies, migrations, configuration/secrets, security-sensitive code, API contract changes, ordinary src/**, releases, hotfixes, and reverts. Each row gives the approvals for develop, the approvals for main, and whether a code-owner approval is required. The document states the tie-break: when a PR spans rows, the strictest row wins and every implicated code owner must approve.
    • §3 Required checks — the two unconditional jobs from .github/workflows/ci.yml (No build artifacts in git and validate, with the individual lint / typecheck / build / migration / schema-drift steps named), the conditionally required checks, and an honest note that the unit/E2E jobs CONTRIBUTING.md §4 references are not yet defined in CI — with the interim rule reviewers must follow until they are.
    • §4 Review validity — stale-approval dismissal on new commits, conversation resolution, branch currency, DCO sign-off, and what happens to approvals after a conflict-resolving rebase.
    • §5 Override rules — who may relax what and under which conditions (§5.1), a seven-item non-overridable list (§5.2) covering build artifacts, the validate job, secrets, security-path code-owner approval, DCO sign-off, branch protection, and direct pushes, the required record in the PR (§5.3), and the emergency-incident path with its 24-hour and 5-business-day follow-ups (§5.4).
    • §6 Hotfixes to main — the conditions for bypassing develop and the mandatory back-merge.
    • §7–§8 — relationship to the other governance and domain documents, and a change log.

Verification Results

Scope check
------------
Files added:      1  (Governance/policies/APPROVAL_REQUIREMENTS.md)
Files modified:   0
Paths outside Governance/: none

Accuracy check against the repository
-------------------------------------
CI jobs: taken from .github/workflows/ci.yml, which defines exactly three jobs:
  'No build artifacts in git', 'validate', 'security-scan'.
  The document quotes these names and the individual validate steps
  (lint:ci, typecheck, build, migrations:check, migration:run,
  migration:generate:check, migration:revert) rather than the CI Passed
  aggregate that CONTRIBUTING.md §4 describes but ci.yml does not define.
Approval counts: rows 1-12 agree with CONTRIBUTING.md §4 (2 on main, 1 on
  develop), with the stricter CODEOWNERS-bearing rows kept at 2 on both.
security-scan status: the pnpm audit step runs with continue-on-error: true for
the #529 phased rollout; the document says so instead of claiming it is gating.
CODEOWNERS: CONTRIBUTING.md §9 shows a CODEOWNERS mapping but no CODEOWNERS file
  is committed. The document says this explicitly and defines the interim rule.

Link check
----------
All 29 relative links in the new document were resolved against the working
path; every one exists on disk, including ../../.github/workflows/ci.yml.

Build impact
------------
Documentation only. Governance/ is outside the Jest root (jest.config.js sets
rootDir: 'src' and testRegex '.*\.spec\.ts$'), outside both TypeScript
programs (tsconfig.json includes only src/** and test/**), and already matched
by the '*.md' rule in .dockerignore. No code, config, schema, or dependency is
touched, so no existing test or build step is affected.

Regression tests
----------------
Not applicable: this is a documentation-only change and adds no executable code.
Acceptance Criteria Status
Governance/policies/APPROVAL_REQUIREMENTS.md created ✅ Added
Minimum approvals per change type documented ✅ §2, twelve change types with per-branch counts and code-owner requirements
Required checks specified ✅ §3 — unconditional jobs named from ci.yml, plus conditional checks and their real gate status
Override rules defined ✅ §5 — who may override what, a seven-item non-overridable list, the recording requirement, and the emergency path
Scope limited to at most two files ✅ One file added, none modified
No changes outside the Governance/ folder ✅ Verified above
No regression in existing functionality ✅ Documentation only; Governance/ is outside the build and test roots
Change is documented ✅ §7 maps the policy against CONTRIBUTING.md, CODEOWNERS, and the domain policies it refines; §8 records the v1.0.0 change

Closes #1600

Adds Governance/policies/APPROVAL_REQUIREMENTS.md: the minimum approvals per
change type, the checks that must be green, review validity rules, and the
authorised override rules including the non-overridable list.

Closes rinafcode#1600
@drips-wave

drips-wave Bot commented Sep 28, 2026

Copy link
Copy Markdown

@abdulraheemabdullahi30-tech Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add PR approval requirements for TeachLink Backend

1 participant