build(config): fix ShipIt pre-release handling, revise merge strategy - #445
Conversation
Add pre_release: alpha to CHANGELOG.md's front matter. Without it, ShipIt drops the -alpha suffix on the next qualifying commit instead of applying a real version bump, promoting GenPRES to a 'stable' release as a side effect of an unrelated fix. GenPRES stays alpha until the MVP work needed to replace the existing system (auth, etc.) is checked off. Refs informedica#234
Rename the latest CHANGELOG.md heading from [0.1.2-alpha] to [0.1.2-alpha.1]. ShipIt's pre-release increment only knows how to bump an already-numbered tag (alpha.N -> alpha.N+1); a bare, unnumbered 'alpha' tag has no '.' for it to find a counter after, and it hits an unhandled 'this should never happen' branch in its own source and crashes instead of starting numbering at 1. Confirmed by dry-running a throwaway fix commit against both the old and new heading. Directory.Build.props still reads 0.1.2-alpha unchanged; nothing outside the changelog heading needs to match yet since ShipIt doesn't read that file until step 4 wires up the xml updater. Refs informedica#234
Concerns were raised about squash-only merging: it discards commit-level history on PRs where that granularity matters. ShipIt's own README treats squash and rebase merging as equally valid, since both avoid the 'Merge pull request ...' commits that break its commit parsing. Revise the decision to disable GitHub's merge-commit option while leaving both squash and rebase enabled, so each contributor chooses per PR instead of one strategy being forced on everyone. Also drops the 'flip this immediately before step 3 merges' urgency from the implementation plan: nothing runs ShipIt unattended until step 5 lands CI integration, and every invocation already passes --skip-merge-commit to tolerate the merge commits already in history, so there's no deadline tied to step 3 specifically. Refs informedica#234
Greptile SummaryThis PR preserves GenPRES’s alpha prerelease sequence and revises the proposed GitHub merge strategy to permit squash or rebase merges while disabling merge commits.
Confidence Score: 4/5The PR appears safe to merge, with a non-blocking rollout-documentation inconsistency around the ShipIt merge-commit option. The prerelease and ADR changes are coherent, but the implementation plan relies on Files Needing Attention: docs/implementation-plans/234-improve-build-system.md Important Files Changed
Reviews (1): Last reviewed commit: "docs(config): revise ADR-0021 merge stra..." | Re-trigger Greptile |
Greptile caught this on PR informedica#445: the implementation plan claims every ShipIt invocation already passes --skip-merge-commit, but the DEVELOPMENT.md example didn't. The bare command crashes on this repo's existing Merge pull request ... commits, as already verified while testing the pre_release fix. Add the flag so the documented command actually works if someone copy-pastes it. Refs informedica#234
|
@ploeh Based on your feedback, we can do either rebase or squash PR's. The only thing to remember is that at PR time (Assuming this is turned on at the CI level), if you use rebase PR, then each commit in the PR would become a bullet point in the changelog, whereas a squash PR merge would be a single entry, albeit probably a fat one. |
|
@halcwb I think the SDK moved from 10.0.302 to 10.0.400, and that might be the cause of the failing test, nothing else pertinent seems to have changed. |
Clarify the build-system design discussion around merge commit policy. The documentation now explains that concerns about losing commit-level history were addressed by disabling merge commits while keeping squash and rebase options available, without forcing a single merge style.
Uh, I really prefer merges (keeping history intact and all that), so if the choice is between squash or rebase, then pick the one you prefer 😀 |
Overview
Three fixes discovered while verifying EasyBuild.ShipIt (adopted in #443) actually
works against this repo's current state, we want to address this befor CI wiring:
pre_release: alphatoCHANGELOG.md's front matter. Without it, the nextqualifying commit would silently drop the
-alphasuffix and promote GenPRES toa "stable" release as a side effect, we don't want this to happen intended until the
MVP work (auth, etc.) is done.
[0.1.2-alpha]to[0.1.2-alpha.1].ShipIt's pre-release increment logic only knows how to bump an already-numbered
tag (
alpha.N -> alpha.N+1); a barealphatag has no number for it to find,and it hits an unhandled branch in its own source and crashes.
squash-only merging (loses commit-level history). ShipIt's own README treats squash and
rebase merging as equally valid, so the decision is now just disabling GitHub's
merge-commit option while leaving both squash and rebase enabled, rather than
forcing squash on everyone.
Refs #234