From ffee99c7df3e8f9df03b6c3d15e5711f431a06db Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adrian=20Padilla=20Vega=20=28m=CC=B5=CD=8A=CD=9Cx=CC=B5?= =?UTF-8?q?=CC=8D=CD=9B=CD=86=CC=8C=CD=83=CC=9B=CC=94=CC=9B=CD=92=CD=9D?= =?UTF-8?q?=CD=82=CD=8B=CC=84=CC=9A=CD=96W=CC=B6=CD=91=CC=AF=CD=85=CD=9C?= =?UTF-8?q?=CC=A4=CC=A7=CC=A0=CC=A3=CC=A8=CC=A8=CD=93=CD=96=CC=B1=CD=99?= =?UTF-8?q?=CD=99o=CC=B7=CC=89=CC=9A=CC=8C=CC=8E=CC=86=CC=BF=CC=94=CD=91?= =?UTF-8?q?=CD=8C=CC=94=CC=92=CC=94=CD=9B=CD=80=CD=86=CC=B1=CC=AC=CD=9A?= =?UTF-8?q?=CC=9E=CC=ACl=CC=B5=CC=94=CC=85=CD=86=CD=94=CC=BA=CD=8E=CC=A2?= =?UTF-8?q?=CC=AD=CC=A8=CC=9Df=CC=B5=CC=86=CC=80=CC=BF=CD=8B=CC=86=CC=85?= =?UTF-8?q?=CD=8A=CD=9D=CC=87=CC=BE=CD=8A=CD=81=CD=89=CD=94=CD=8D=CC=AC?= =?UTF-8?q?=CD=95=CD=89=CD=99=CC=B9=CD=88=CC=98=CD=88=29?= Date: Wed, 23 Sep 2026 17:29:35 -0700 Subject: [PATCH] feat: adopt a trunk-based workflow with merge commits on main MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `main` becomes the only long-lived branch. Short-lived branches carry one pull request each and are deleted after merging, so `develop` is no longer a stepping stone and its ruleset is gone — a feature branch needs no protection, since it cannot reach `main` except through the CI-gated pull request. `main` now takes merge commits instead of squashes, to preserve authorship: a merge commit adds one object and leaves the branch's commits untouched, so each keeps the signature its author made, while GitHub signs the merge commit itself to satisfy `required_signatures`. Squashing would have collapsed them into one GitHub-signed commit, discarding the author's signatures. The cost is that `main` is no longer linear. CI triggers narrow to `main` on both stubs. The pull_request trigger needs no source-branch list — it fires for a PR from any branch — and leaving feature branches out of the push trigger avoids building the same commits twice. Also documents that `rulesets-apply` never deletes, which is why removing develop-protection.json did not remove the ruleset from GitHub. --- .github/rulesets/README.md | 70 +++++++++++++++--------- .github/rulesets/develop-protection.json | 52 ------------------ .github/rulesets/main-protection.json | 2 +- .github/workflows/ci.yml | 12 +++- README.md | 50 ++++++++++------- azure-pipelines.yml | 6 +- 6 files changed, 89 insertions(+), 103 deletions(-) delete mode 100644 .github/rulesets/develop-protection.json diff --git a/.github/rulesets/README.md b/.github/rulesets/README.md index 62b5f8a..bd1db9d 100644 --- a/.github/rulesets/README.md +++ b/.github/rulesets/README.md @@ -19,36 +19,40 @@ workflow they impose — how a change actually reaches `main` — see ## What the shipped rulesets enforce -| | `main-protection` | `develop-protection` | -| --- | --- | --- | -| Enforcement | `active` | `disabled` | -| Direct pushes | blocked — pull request required | blocked when enabled | -| Required check | `ci` (GitHub Actions) | `ci` (GitHub Actions) | -| Branch must be up to date | yes | yes | -| Approvals | 1, admins may bypass in a PR | 1 | -| Merge methods | squash only (see below) | merge, squash, rebase | -| Signed commits | required | not required | -| Force push / deletion | blocked | blocked | - -`develop-protection` ships **disabled** so that a new project can commit -straight to `develop` while it finds its feet. Flip `enforcement` to `active` -in the JSON and run `make rulesets-apply` when you want pull requests there too. - -Two consequences of `main-protection` worth knowing before you enable it on a -project: +`main-protection` is the only ruleset here, because the workflow is +**trunk-based**: `main` is the sole long-lived branch, and everything else is a +short-lived branch that exists only long enough to carry one pull request. There +is no `develop` stepping stone, so there is nothing else to protect — a feature +branch needs no rules, since it cannot reach `main` except through a reviewed, +CI-gated pull request. + +| `main-protection` | | +| --- | --- | +| Enforcement | `active` | +| Direct pushes | blocked — pull request required, from any branch | +| Required check | `ci` (GitHub Actions) | +| Branch must be up to date | yes | +| Approvals | 1, admins may bypass in a PR | +| Merge methods | merge commits only (see below) | +| Signed commits | required | +| Force push / deletion | blocked | + +Three consequences worth knowing before you enable this on a project: - **Merges must happen through GitHub.** Direct pushes to `main` are blocked, and `required_signatures` means whatever lands there must be signed. GitHub signs - the commits it creates for a squash or a merge, so those satisfy the rule. A - local push will be rejected. -- **`required_signatures` rules out rebase merges.** A rebase merge rewrites each - commit into a new object, discarding the author's signature, and GitHub cannot - re-sign on the author's behalf. Listing `rebase` in `allowed_merge_methods` - alongside this rule yields a merge button that always fails with *"Base branch - requires signed commits. Rebase merges cannot be automatically signed by - GitHub."* That is why `main` allows squash only. If you want rebase merges and - a linear history of individual commits, you have to drop `required_signatures` - — the two cannot coexist. + the merge commit it creates, which satisfies the rule. A local push will be + rejected. +- **The merge method is a merge commit, chosen to preserve authorship.** A merge + commit adds one new object and leaves the branch's own commits untouched, so + each keeps the signature its author made. Squash and rebase both rewrite + history: a squash collapses the branch into one new GitHub-signed commit, and a + rebase rewrites every commit — which *discards* the author's signature, leaving + GitHub nothing to re-sign with. Listing `rebase` alongside + `required_signatures` yields a merge button that always fails with *"Base + branch requires signed commits. Rebase merges cannot be automatically signed by + GitHub."* The cost of merge commits is that `main` is not linear; that is the + deliberate trade for keeping author signatures in the history. - **The admin bypass is real.** `bypass_mode: "pull_request"` lets an admin merge a pull request whose `ci` check is red. It prevents an accidental merge, not a deliberate one. Remove the `bypass_actors` entry if you want the check to be @@ -101,3 +105,15 @@ belongs in this file. If you change protection in the GitHub UI, run `make rulesets-export` to bring the change back into the repo; otherwise the next `make rulesets-apply` will silently revert it. + +**`apply` never deletes.** It creates and updates the rulesets named by the files +here, and ignores anything else on the repo. Removing a file therefore leaves its +ruleset live on GitHub — delete that one yourself: + +```sh +id=$(gh api repos/OWNER/REPO/rulesets --jq '.[] | select(.name=="NAME") | .id') +gh api --method DELETE repos/OWNER/REPO/rulesets/$id +``` + +The omission is deliberate: a prune step would give a script that runs against +any repo the power to remove protection it did not create. diff --git a/.github/rulesets/develop-protection.json b/.github/rulesets/develop-protection.json deleted file mode 100644 index 74049f3..0000000 --- a/.github/rulesets/develop-protection.json +++ /dev/null @@ -1,52 +0,0 @@ -{ - "bypass_actors": [], - "conditions": { - "ref_name": { - "exclude": [], - "include": [ - "refs/heads/develop" - ] - } - }, - "enforcement": "disabled", - "name": "develop-protection", - "rules": [ - { - "type": "deletion" - }, - { - "type": "non_fast_forward" - }, - { - "parameters": { - "allowed_merge_methods": [ - "merge", - "squash", - "rebase" - ], - "dismiss_stale_reviews_on_push": false, - "require_code_owner_review": true, - "require_extra_approval_for_unattributed_changes": true, - "require_last_push_approval": false, - "required_approving_review_count": 1, - "required_review_thread_resolution": false, - "required_reviewers": [] - }, - "type": "pull_request" - }, - { - "parameters": { - "do_not_enforce_on_create": false, - "required_status_checks": [ - { - "context": "ci", - "integration_id": 15368 - } - ], - "strict_required_status_checks_policy": true - }, - "type": "required_status_checks" - } - ], - "target": "branch" -} diff --git a/.github/rulesets/main-protection.json b/.github/rulesets/main-protection.json index fab10ea..de9654d 100644 --- a/.github/rulesets/main-protection.json +++ b/.github/rulesets/main-protection.json @@ -29,7 +29,7 @@ { "parameters": { "allowed_merge_methods": [ - "squash" + "merge" ], "dismiss_stale_reviews_on_push": true, "require_code_owner_review": false, diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9ff27f..946906d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -22,10 +22,18 @@ name: ci on: + # Trunk-based: `main` is the only long-lived branch. Feature branches are + # short-lived and reach `main` only through a pull request, so the + # pull_request trigger is the gate — it fires for a PR from ANY head branch, + # which is why no head-branch list is needed here. + # + # The push trigger covers `main` only, deliberately: adding feature branches + # would run the same commits twice (once for the push, once for the PR) for no + # extra signal. push: - branches: [main, develop] + branches: [main] pull_request: - branches: [main, develop] + branches: [main] jobs: # The job id is the name GitHub reports this check under, so it is also the diff --git a/README.md b/README.md index 9702852..d4e25eb 100644 --- a/README.md +++ b/README.md @@ -158,8 +158,12 @@ portable script: `make ci`. - **Triggers** (which branches/events run CI) — expressed differently on each platform. Both stubs ship pre-configured to run CI on: - - **pushes** to `main` and `develop`, and - - **pull requests** targeting `main` and `develop`. + - **pushes** to `main`, and + - **pull requests** targeting `main`, from any source branch. + + Feature branches are intentionally absent from the push trigger: a branch that + has an open pull request would otherwise build the same commits twice for no + extra signal. The pull-request trigger must cover every branch you gate with a required status check (see [Branch protection](#branch-protection-github-rulesets)). @@ -224,31 +228,39 @@ the shipped rulesets enforce. ### The workflow these rules require +The workflow is **trunk-based**. `main` is the only long-lived branch: there is +no `develop` to stage through, and no release branches. Everything else is a +short-lived branch that exists just long enough to carry one pull request, and +is deleted after it merges. + With `main-protection` active, `main` cannot be written to directly. Every change reaches it the same way: -1. **Work on `develop`** (or a branch off it). Commits are checked locally by - the hooks from `make setup`, and pushing runs `make ci` on the server. - `develop-protection` ships **disabled**, so committing straight to `develop` - is allowed until you enable it. +1. **Branch off `main`.** Name it however you like — the rules place no + constraint on source branches, and a feature branch needs no protection of + its own, since it cannot reach `main` except through the gate below. Keep it + short-lived; the point of trunk-based work is that branches merge in days, + not weeks. Commits are checked locally by the hooks from `make setup`. 2. **Open a pull request into `main`.** A direct `git push origin main` is rejected by the ruleset, as is a force-push and a branch deletion. 3. **Let `ci` finish and pass.** It is a required check, so the merge button stays disabled until it reports success. The branch must also be up to date with `main` first (`strict_required_status_checks_policy`), so if `main` - moved, rebase and let `ci` run again. -4. **Merge with squash.** `main` accepts *only* squash merges — the merge and - rebase buttons are not offered. GitHub signs the single commit it creates, - which is what satisfies `required_signatures`; a local push cannot land on - `main` at all. - - **Rebase merges are deliberately not allowed, and cannot be.** A rebase merge - rewrites each commit into a new object, which discards the author's - signature, and GitHub has no key with which to re-sign on the author's - behalf. Enabling `rebase` alongside `required_signatures` produces a merge - button that always fails with *"Base branch requires signed commits. Rebase - merges cannot be automatically signed by GitHub."* The two rules are - mutually exclusive, so pick which one matters more before changing this. + moved, update the branch and let `ci` run again. +4. **Merge with a merge commit, then delete the branch.** `main` accepts *only* + merge commits — squash and rebase are not offered. That choice is about + authorship: a merge commit leaves your commits untouched, so each keeps the + signature you made, and GitHub signs the merge commit itself to satisfy + `required_signatures`. The trade is that `main` is not linear. + + **Rebase merges are not allowed, and cannot be.** A rebase merge rewrites + each commit into a new object, which discards the author's signature, and + GitHub has no key with which to re-sign on the author's behalf. Enabling + `rebase` alongside `required_signatures` produces a merge button that always + fails with *"Base branch requires signed commits. Rebase merges cannot be + automatically signed by GitHub."* A squash merge does work, but it collapses + the branch into one new GitHub-signed commit, so your own signatures do not + survive onto `main`. What this buys you, and what it does not: the rules keep a red build from reaching `main` by accident. They are not a hard stop, because the shipped diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 25b03fd..63b5061 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -21,17 +21,19 @@ # requirements-dev.txt; hook versions via `rev` in .pre-commit-config.yaml). # Bump them deliberately. +# Trunk-based: `main` is the only long-lived branch. The `pr` trigger is the +# gate and fires for a pull request from any source branch. `trigger` covers +# `main` only, deliberately: adding feature branches would run the same commits +# twice for no extra signal. trigger: branches: include: - main - - develop pr: branches: include: - main - - develop pool: # Pinned rather than `ubuntu-latest`: that label is remapped to a new Ubuntu