ci: harden and document the release procedure#268
Conversation
🦋 Changeset detectedLatest commit: 6e53362 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
I understand that there are no changesets, but there should be one, or at least there should be a recommendation about the version impact. In a perfect world, we would at least see the next version calculated based on conventional commits and semantic release conventions. So in this case, there are two commits that contain the prefix fix. And for those, that should trigger a patch increase. Now obviously when the PR lands, it would be a single patch level increase, not multiple patch level increases. I also wonder what happens if we don't have a change set and instead we decide to roll together n number of changes in a specific build. In that case, I suppose the next time we went to go do a change set, it would calculate and find the highest version bump that was available. The problem with that is that now we have a batching problem. In lean software engineering, you'd want to be releasing more often. So the fact that there is a PR without a changeset to me feels like a smell or as possibly an anti pattern. I'm willing to have the conversation and there are no right answers here. There are just a bunch of potentially wrong ones. So please take this as more of a, let's think about this – this is feedback v "sosmething here is wrong." I think this is suboptimal right now, and I think that we can probably improve it. Those of us who are handling releases have the confidence that we're doing the right thing for each individual PR and that we're shipping improvements to the community as fast as humanly possible and as responsibly as our mission demands. For things like patch releases that may address small issues (and don't necessarily rise to the level of a point increase), most consumers will allow patch level increases for things like security fixes, or small bugs. But even we shouldn't wholesale be adopting point level increases, especially when we consider things like supply chain attacks. Small PRs that are patch fixes and bug fixes tend to get approved as long as the change set is relatively small. The size of a change and its "reviewability" are a signal about a given change's impact, and smaller changes should receive careful-enough scrutiny to avoid loosening security or introducing other subtle bugs, especially since so many of us allow these patch-level increases automatically (like Dependabot, e.g.) Things like point increases should trigger much more diligence. Our downstream dependencies and their transitive dependencies may have wider wider version ranges that they accept. But generally speaking, in this day and age, we should be pinning specific versions and maybe allowing for patch increases. And we should probably be telling our consumers and partners to do the same. |
jhampton
left a comment
There was a problem hiding this comment.
Please take the feedback and make sure it's captured in this repo as learning (agents, skills, etc). Some of these need @davidfedor consulted if not deciding. We can (and should) defer landing this until we have clear decisions, then pin those behaviors with both deterministic (code/config) guardrails and semi-deterministic (Greptile, AGENTS.md) rules.
jhampton
left a comment
There was a problem hiding this comment.
Resolved the two questions. Lookin' good.
578c2b4 to
7f7ece4
Compare
…ardening # Conflicts: # pnpm-lock.yaml
Ports the release-hardening work from YPE-2486 to the React SDK.
What's here
origin/maintip, huskycommit-msghook, and config (allows theYPE-####prefix, ignores the release commit). Ported from the Kotlin SDK.Greptile Summary
This PR hardens the release process by adding CI gates, corepack-aware git hooks, and comprehensive operator documentation. The changes are primarily CI/tooling with no package source modifications.
changeset.yml), conventional PR title format (pr-title.yml), and per-commit lint via a merge-base-anchored range (commitlint.yml), using Node 24 and commitlint@19.8.1to satisfy the>=22.13engine floor..husky/pre-commitand the new.husky/commit-msgnow prefercorepack pnpm ...with a plainpnpmfallback, per the documented Decision 2 rationale indocs/release-hardening-decisions.md.RELEASE-RUNBOOK.mdcatalogues 10 failure modes with state-check and recovery commands;docs/release-hardening-plan.mdanddocs/release-hardening-decisions.mdrecord the engineering decisions and keep-Changesets rationale.Confidence Score: 5/5
Safe to merge — all changes are CI workflows, git hooks, and documentation with no package source modifications.
No package code is touched. The three new GitHub Actions workflows are logically correct: the changeset gate uses a proper merge-base diff and excludes the release branch; the commitlint range is anchored to the live origin/main tip; the PR-title workflow is straightforward. The husky hooks follow the documented corepack-preferred / pnpm-fallback pattern. The commitlint dependency was pinned to @19.8.1 to satisfy the >=22.13 engine floor. The only findings are documentation nits — a missing permissions block in commitlint.yml and a stale status in the Decision 4 section of the decisions doc.
docs/release-hardening-decisions.md — the Decision 4 section body still reads as unresolved despite this PR shipping the gate.
Important Files Changed
permissions:block unlike the other two new workflows.Flowchart
%%{init: {'theme': 'neutral'}}%% flowchart TD PR[Pull Request opened / updated] --> PT[pr-title.yml\namannn/action-semantic-pull-request\nConventional Commits check on title] PR --> CL[commitlint.yml\nNode 24 · pnpm install\ngit merge-base origin/main HEAD\ncommitlint --from base --to head] PR --> CS[changeset.yml\nfetch-depth 0\ngit merge-base origin/main HEAD\ndiff .changeset *.md vs base] PT -->|title valid| PT_OK[✅ PR title gate passes] PT -->|invalid| PT_FAIL[❌ Block merge] CL -->|all commits pass| CL_OK[✅ Commitlint gate passes] CL -->|violation found| CL_FAIL[❌ Block merge] CS -->|changeset added| CS_OK[✅ Changeset gate passes] CS -->|no changeset| CS_FAIL[❌ Block merge\nRun pnpm changeset or\npnpm changeset --empty] PT_OK & CL_OK & CS_OK --> MERGE[Merge to main] MERGE --> REL[release.yml\nchangesets/action\nVersion Packages PR or publish]Reviews (13): Last reviewed commit: "Merge branch 'main' into ks/YPE-2486-rel..." | Re-trigger Greptile
Context used (4)