From bc0706cf363fe02a1634edbfdb04c42d447da8ee Mon Sep 17 00:00:00 2001 From: Attila Szegedi Date: Tue, 11 Aug 2026 12:47:15 +0200 Subject: [PATCH] docs(release): classify branch-diff false positives branch-diff matches commits rather than content, so it reports commits whose changes are already on v5.x. Two thirds of its output for v5.18.0 was noise. The skill previously said only "skip commits that would result in empty cherry-picks", which gives no way to tell those apart from real ones. Document the three classes actually observed: a. Commits subsumed by the squash-merged 5.14.2/5.14.3/5.14.4 releases. Enumerated per release; a closed set that will not grow. b. Dependabot bumps superseded by a later bump of the same package on v5.x. Cherry-picking one downgrades the branch. c. #154, the 6.0.0-pre bump on main, which must never reach a 5.x branch. Applying these to `branch-diff v5.x main` yields exactly the 13 commits in the v5.18.0 proposal. Also add a `git diff --stat main` check before the version bump. An age-based cutoff had dismissed #352 as a false positive when it was real and unapplied; the content diff is what exposed it, so the skill now states that age alone is not evidence and cites #352 as the counterexample. Smaller fixes for things that misled during v5.18.0: pull both branches before comparing, parse PR numbers from the trailing URL rather than the "(#NNN)" form (which false-matches PR references in commit titles), clear the previous release's worktree, and keep the version commit last on the branch. --- .claude/skills/release/SKILL.md | 78 ++++++++++++++++++++++++++++++--- 1 file changed, 73 insertions(+), 5 deletions(-) diff --git a/.claude/skills/release/SKILL.md b/.claude/skills/release/SKILL.md index b96de691..2ff29356 100644 --- a/.claude/skills/release/SKILL.md +++ b/.claude/skills/release/SKILL.md @@ -15,6 +15,13 @@ The `branch-diff` tool must be installed globally: npm install branch-diff -g ``` +Fetch and fast-forward **both** branches before doing anything else. Comparing a +stale `v5.x` against a stale `main` silently produces a wrong commit list: + +``` +git fetch origin && git checkout v5.x && git pull && git checkout main && git pull +``` + ## Steps ### 1. Identify commits to cherry-pick @@ -25,9 +32,36 @@ Use the `branch-diff` tool to list commits on `main` not yet applied to `v5.x`: branch-diff v5.x main ``` -Review the output with the user. Skip: -- Version bump commits (e.g. "Bump package version on to 6.0.0-pre") -- Commits that would result in empty cherry-picks (already applied or superseded) +Its GitHub issue-lookup errors go to stderr; the commit list is on stdout. PR numbers +appear in the trailing URL (`.../pull/393`), *not* as `(#393)` — parsing the `(#NNN)` +form instead picks up PR references that happen to appear in commit titles. + +`branch-diff` matches commits, not content, so it reports a substantial number of +**false positives** — commits whose changes are already on `v5.x`. Do not cherry-pick +these. They fall into three classes: + +**a. Squash-merged releases.** Releases 5.14.2, 5.14.3 and 5.14.4 were squash-merged +rather than rebased, so every commit they contained lost its identity and is reported +forever. This set is closed and will not grow — treat all of these as already released: + +| Release | Proposal | PRs subsumed | +|---|---|---| +| 5.14.2 | #331 | 284, 310, 311, 315, 316, 317, 320, 323, 324, 325, 326, 327, 329 | +| 5.14.3 | #334 | 328, 332 | +| 5.14.4 | #337 | 333, 335, 336 | + +**b. Superseded dependency bumps.** A Dependabot bump that never landed on `v5.x`, which +later picked up an equal-or-newer version of the same package directly. Cherry-picking one +would *downgrade* the branch. Recognise these by comparing the package version in +`v5.x:package.json` against the bump's target — skip when `v5.x` is at or ahead of it. +(Examples seen so far: #140, #344, #348, #349, #350.) + +**c. The `main`-only version bump.** #154 moved `main` to `6.0.0-pre`. It must never be +cherry-picked onto a 5.x release branch. + +Anything left after removing those three classes is a genuine candidate. Note that being +old is *not* by itself evidence of a false positive: #352 sat below all of these and was a +real, unapplied commit. Classify by the rules above, not by age. Confirm the list of commits with the user before proceeding. @@ -54,6 +88,13 @@ Create a git worktree from the current repo, checking out a new branch `v$VERSIO git worktree add ../pprof-nodejs-v5 -b v$VERSION-proposal v5.x ``` +The path is usually still occupied by the previous release's worktree. Once that +proposal's PR is merged, it is safe to clear — verify it is clean and merged first, then: + +``` +git worktree remove ../pprof-nodejs-v5 && git branch -D v-proposal +``` + All subsequent steps run in the worktree directory. ### 4. Cherry-pick commits @@ -66,7 +107,30 @@ git cherry-pick ... If a cherry-pick has conflicts, stop and resolve with the user. -### 5. Create the version bump commit +### 5. Verify the selection against `main` + +Before bumping the version, diff the worktree against `main`: + +``` +git diff --stat main -- . +``` + +The goal is **minimal divergence**: ideally this reports nothing but `package.json` and +`package-lock.json` (the version, plus any dev-dep bump this release includes). + +This is the check that validates step 1, and it is worth doing carefully — it is how #352 +was caught, a genuinely unapplied commit that a plausible-looking age heuristic had +written off as a false positive. Any *other* file appearing here means one of two things: + +- a real commit was wrongly classified as a false positive — cherry-pick it, or +- the divergence is deliberate — say so explicitly in the PR body rather than leaving it + silently unexplained. + +Note that a class-(b) superseded bump correctly shows up as a `package.json` / +`package-lock.json` difference where `v5.x` is *ahead* of `main`. That is expected and +should be left alone. + +### 6. Create the version bump commit Bump the version in package.json and package-lock.json using npm, then commit: @@ -76,7 +140,11 @@ git add package.json package-lock.json git commit -m "v$VERSION" ``` -### 6. Push and create a PR +Keep this commit last on the branch. If a further cherry-pick turns out to be needed after +this point, drop the version commit (`git reset --hard HEAD~1`), apply the cherry-pick, +then re-run the bump — rather than stacking the new commit on top of the release commit. + +### 7. Push and create a PR Push the branch and create a PR targeting `v5.x`: