fm-pr-merge: make the merge method configurable, and document why squash is load-bearing #11
Loading…
Reference in a new issue
No description provided.
Delete branch "fm/fm-merge-default-merge-commit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
What this changes
bin/fm-pr-merge.shhard-coded--squash. It is now a configurable default that still defaults to squash, with--mergeand--rebaseavailable explicitly, and the reason for that default is written down where the next reader will find it.Why the default did NOT become a merge commit, which is what was originally asked for
The captain asked for merge commits, on the reasonable grounds that squashing destroys history and offers no benefit beyond it. Partway through this work, a live merge attempt settled the question:
pyr0-dev-infraprotectsmainwith a signed-commits requirement. A squash merge satisfies it because Forgejo creates a new commit signed with the instance key. A merge commit does not, because it carries the branch's own unsigned commits ontomain. The identical merge succeeded as a squash immediately afterwards.So squashing on that repository is not a style preference — it is what makes the protection satisfiable. A default flip would have broken merging there outright, and would break it on any repository that later adopts signing. Since the stated long-term intent is signed commits everywhere, hard-coding merge commits would have been wrong in exactly the direction the fleet is heading.
An earlier iteration of this branch did flip the default, passed its pipeline, and was caught by reading the diff rather than by any check. That is worth recording.
The real trade-off, unresolved and not resolved here
Squashing is what removed the revertable unit behind a defect found the same day: a rollback runbook instructed
git reverton a Dockerfile pin that had landed inside a nine-file squash commit, so following it during an incident would have deleted the runbook itself.Both concerns are legitimate and they conflict. They only stop conflicting if worker commits themselves are signed — at which point merge commits become possible under a signed-everywhere policy and history stays revertable. That is a fleet decision, not a change to this script, and it is deliberately left open.
What else is in here
--mergeand--rebasefail loudly with the forge's real reason when they cannot land, rather than silently falling back to squash. That loud failure is what made this whole finding visible in the first place; silently succeeding would have hidden it.docs/configuration.mdnow state the default and why it is what it is, so nobody "simplifies" it back.Revert the default from --merge back to --squash: a live Forgejo rejection ('branch main is protected from unverified commit') proved squash is what satisfies signed-commit branch protection, because the forge signs the single new commit whereas a merge commit or rebase carries the branch's unsigned commits onto main. It is not a style preference. Add a home-wide config/merge-default override (squash, merge, rebase) read when the caller passes no explicit method; an invalid value fails loudly. Explicit --squash/--merge/--rebase still win outright and a rejected explicit method surfaces the forge's reason without silently falling back to squash. Update the header and docs to state the default and why, and extend the tests to pin the squash default, the config override, explicit --rebase, and the no-fallback failure path.