From fb252460519e2972becb3785eb806da212bd7e7e Mon Sep 17 00:00:00 2001 From: argenis de la rosa Date: Wed, 4 Mar 2026 13:47:43 -0500 Subject: [PATCH] docs(governance): formalize no-squash contributor attribution policy --- docs/pr-workflow.md | 13 +++++++++---- docs/reviewer-playbook.md | 8 ++++++++ 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/docs/pr-workflow.md b/docs/pr-workflow.md index a69302e4c..7f21b7269 100644 --- a/docs/pr-workflow.md +++ b/docs/pr-workflow.md @@ -96,12 +96,14 @@ Automation assists with triage and guardrails, but final merge accountability re Maintain these branch protection rules on `dev` and `main`: - Require status checks before merge. -- Require check `CI Required Gate`. +- Require checks `CI Required Gate` and `Security Required Gate`. - Require pull request reviews before merge. +- Require at least 1 approving review. - Require CODEOWNERS review for protected paths. - For `.github/workflows/**`, require owner approval via `CI Required Gate` (`WORKFLOW_OWNER_LOGINS`) and keep branch/ruleset bypass limited to org owners. - Default workflow-owner allowlist includes `theonlyhennygod`, `willsarg`, and `chumyin` (plus any comma-separated additions from `WORKFLOW_OWNER_LOGINS`). - Dismiss stale approvals when new commits are pushed. +- Keep `require_last_push_approval` disabled so one maintainer approval can satisfy merge policy. - Restrict force-push on protected branches. - Route normal contributor PRs to `main` by default (`dev` is optional for dedicated integration batching). - Allow direct merges to `main` once required checks and review policy pass. @@ -125,7 +127,7 @@ Maintain these branch protection rules on `dev` and `main`: ### 4.2 Step B: Validation -- `CI Required Gate` is the merge gate. +- `CI Required Gate` and `Security Required Gate` are the merge gates. - Docs-only PRs use fast-path and skip heavy Rust jobs. - Non-doc PRs must pass lint, tests, and release build smoke check. - Rust-impacting PRs use the same required gate set as `dev`/`main` pushes (no PR build-only shortcut). @@ -138,7 +140,10 @@ Maintain these branch protection rules on `dev` and `main`: ### 4.4 Step D: Merge -- Prefer **squash merge** to keep history compact. +- Keep **squash merge disabled** to preserve contributor commit attribution. +- Prefer **merge commit** for normal contributor PRs. +- Allow **rebase merge** when commits are already clean and linear history improves reviewability. +- Maintainer approval is required before merge, but approval should not rewrite or replace contributor authorship. - PR title should follow Conventional Commit style. - Merge only when rollback path is documented. @@ -157,7 +162,7 @@ Maintain these branch protection rules on `dev` and `main`: ### 5.2 Definition of Done (DoD) merge-ready -- `CI Required Gate` is green. +- `CI Required Gate` and `Security Required Gate` are green. - Required reviewers approved (including CODEOWNERS paths). - Risk class labels match touched paths. - Migration/compatibility impact is documented. diff --git a/docs/reviewer-playbook.md b/docs/reviewer-playbook.md index 3a0285695..d19568982 100644 --- a/docs/reviewer-playbook.md +++ b/docs/reviewer-playbook.md @@ -119,6 +119,14 @@ Prefer checklist-style comments with one explicit outcome: Avoid vague comments that create avoidable back-and-forth latency. +### 3.5 Contribution attribution and merge method + +- Do not squash contributor PRs; squash merge is disabled by repository policy. +- Prefer merge commit to preserve original commit authorship on contributor work. +- Rebase merge is allowed when commit history is already clean and no attribution is lost. +- Keep maintainer role focused on review and approval; do not rewrite contributor commits unless a fix is required. +- If maintainer follow-up commits are required, keep contributor commits intact and avoid replacing authorship history. + --- ## 4. Issue Triage and Backlog Governance