From dcc6488fff0502e94c7e128529486868de754e94 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sat, 17 Oct 2026 09:57:39 +0000 Subject: [PATCH 1/2] Derive changelog PR links from the squash commit Fragments no longer need a PR link, so new PRs don't need the placeholder. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NP47qMfjMbjUTKZDsLthDo Signed-off-by: Christoph Knittel --- AGENTS.md | 2 +- changes/README.md | 6 +- .../changelog-derived-pr-links.internal.md | 1 + scripts/changelog.js | 82 ++++++++++++-- scripts/changelog.test.js | 105 +++++++++++++++++- 5 files changed, 182 insertions(+), 14 deletions(-) create mode 100644 changes/changelog-derived-pr-links.internal.md diff --git a/AGENTS.md b/AGENTS.md index 1844abe36a..770d145c4e 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -100,7 +100,7 @@ on `master`. CI runs on every PR in the stack. ## Testing and changelog -**Add a changelog fragment** `changes/..md` for any user-facing change (bug fix, feature, or breaking change) instead of editing `CHANGELOG.md`; see [changes/README.md](changes/README.md) for categories and format. Validate with `yarn changelog:check`. Since the PR number is not known yet, open the PR as a draft with the placeholder link `https://github.com/rescript-lang/rescript/pull/XXXX`, then replace it with the real link, amend, push again, and mark the PR ready for review (`gh pr ready`). +**Add a changelog fragment** `changes/..md` for any user-facing change (bug fix, feature, or breaking change) instead of editing `CHANGELOG.md`; see [changes/README.md](changes/README.md) for categories and format. Validate with `yarn changelog:check`. Fragments need no PR link: the release adds it from the squash commit that merged the fragment. For compiler or standard-library changes, run `make test`. For syntax changes, also run `make test-syntax`; use `make test-syntax-roundtrip` when parsing or diff --git a/changes/README.md b/changes/README.md index 3f8501bc5e..0bfdb45720 100644 --- a/changes/README.md +++ b/changes/README.md @@ -3,10 +3,12 @@ Add a unique `..md` file for each change that needs release notes. For example, `fix-watcher-recovery.fix.md`: ```markdown -- Recover blocked dependents after a failed build. https://github.com/rescript-lang/rescript/pull/8667 +- Recover blocked dependents after a failed build. ``` -The PR number is only known once the PR exists, so open it as a draft with the placeholder link `https://github.com/rescript-lang/rescript/pull/XXXX`, replace the placeholder with the real link, amend and push again, then mark the PR ready for review. The changelog check fails while the placeholder remains. Each file may contain multiple bullets in the same category. Indent wrapped lines by two spaces; each bullet ends with one or more PR links. +Each file may contain multiple bullets in the same category. Indent wrapped lines by two spaces. + +A bullet needs no PR link: `yarn changelog:release` appends the link of the pull request that added the fragment, taken from the subject of its squash commit (`... (#8667)`). Pull requests are therefore squash-merged, keeping the `(#N)` GitHub adds to the title. A bullet may still end with explicit PR links, for example for a change spanning several pull requests, and those are kept as written. After each push to `master`, the changelog check fails if a fragment without a link cannot be traced to its pull request; add the link to that fragment. | Category | Section | | --- | --- | diff --git a/changes/changelog-derived-pr-links.internal.md b/changes/changelog-derived-pr-links.internal.md new file mode 100644 index 0000000000..9b98cbbe1f --- /dev/null +++ b/changes/changelog-derived-pr-links.internal.md @@ -0,0 +1 @@ +- Fragments no longer need a PR link; the release adds it from the squash commit that merged the fragment. diff --git a/scripts/changelog.js b/scripts/changelog.js index 25081bf7d4..ee15c8045f 100644 --- a/scripts/changelog.js +++ b/scripts/changelog.js @@ -19,6 +19,7 @@ export const placeholderLink = const trailingPrLinks = new RegExp( `${prLink.source}(?:\\s+${prLink.source})*$`, ); +const repositoryUrl = "https://github.com/rescript-lang/rescript"; export function parseFragment(name, content) { const match = /^([a-z0-9][a-z0-9-]*)\.([a-z]+)\.md$/.exec(name); @@ -37,7 +38,6 @@ export function parseFragment(name, content) { const invalid = bullets.some( bullet => !/^- \S/.test(bullet) || - !trailingPrLinks.test(bullet) || bullet .split("\n") .slice(1) @@ -45,13 +45,59 @@ export function parseFragment(name, content) { ); if (!text || invalid) { throw new Error( - `${name}: expected Markdown bullets ending with PR links (indent continuation lines by two spaces)`, + `${name}: expected Markdown bullets (indent continuation lines by two spaces)`, ); } - const pr = Math.max( - ...[...text.matchAll(new RegExp(prLink, "g"))].map(link => Number(link[1])), + const links = [...text.matchAll(new RegExp(prLink, "g"))].map(link => + Number(link[1]), ); - return { name, category: match[2], text, pr }; + // A bullet without a link gets the link of the pull request that added the + // fragment when the release is prepared. + const unlinked = bullets.some(bullet => !trailingPrLinks.test(bullet)); + const pr = links.length ? Math.max(...links) : null; + return { name, category: match[2], text, pr, unlinked }; +} + +// The pull request that added a fragment, from the subject of the squash +// commit that added it ("... (#1234)"). Rebase merges and merge commits carry +// no such subject, which is why pull requests are squash-merged. +export function addedByPullRequest(root, name) { + const subjects = execFileSync( + "git", + ["log", "--diff-filter=A", "--format=%s", "--", `changes/${name}`], + { cwd: root, encoding: "utf8" }, + ) + .split("\n") + .filter(Boolean); + // The most recent addition, in case an earlier fragment had the same name. + const match = /\(#([1-9]\d*)\)\s*$/.exec(subjects[0] ?? ""); + return match ? Number(match[1]) : null; +} + +// Appends the link of the pull request that added the fragment to each bullet +// without one. +export function linkFragment(fragment, pullRequestOf) { + if (!fragment.unlinked) return fragment; + const pr = pullRequestOf(fragment.name); + if (!pr) { + throw new Error( + `changes/${fragment.name}: no PR link, and the commit that added it does not name its pull request; add the link to the fragment`, + ); + } + const text = fragment.text + .split(/\n(?=- )/) + .map(bullet => + trailingPrLinks.test(bullet) + ? bullet + : `${bullet} ${repositoryUrl}/pull/${pr}`, + ) + .join("\n"); + return { + ...fragment, + text, + pr: Math.max(pr, fragment.pr ?? 0), + unlinked: false, + }; } export async function readFragments(root) { @@ -76,12 +122,17 @@ export async function readFragments(root) { return fragments; } -// Newest PR first, matching the order of existing CHANGELOG.md entries. +// Newest PR first, matching the order of existing CHANGELOG.md entries. A +// fragment without a link yet belongs to an unmerged pull request, so it comes +// first. function categoryNotes(fragments, category) { + const order = fragment => fragment.pr ?? Number.POSITIVE_INFINITY; return fragments .filter(fragment => fragment.category === category) .sort( - (a, b) => b.pr - a.pr || (a.name < b.name ? -1 : a.name > b.name ? 1 : 0), + (a, b) => + Math.sign(order(b) - order(a) || 0) || + (a.name < b.name ? -1 : a.name > b.name ? 1 : 0), ) .map(fragment => fragment.text); } @@ -217,7 +268,11 @@ export function checkPullRequest(files, pullRequest) { } } -export async function run(command, root = process.cwd()) { +export async function run( + command, + root = process.cwd(), + pullRequestOf = name => addedByPullRequest(root, name), +) { if (!["check", "preview", "release", "open"].includes(command)) { throw new Error( "Usage: node scripts/changelog.js check|preview|release|open", @@ -245,6 +300,10 @@ export async function run(command, root = process.cwd()) { files.push({ status: diff[i], name: diff[i + 1] }); } checkPullRequest(files, pullRequest); + } else if (process.env.GITHUB_EVENT_NAME === "push") { + // Once merged, every fragment must resolve to its pull request, so a + // merge without "(#N)" in its subject fails here rather than at release. + for (const fragment of fragments) linkFragment(fragment, pullRequestOf); } return; } @@ -261,13 +320,16 @@ export async function run(command, root = process.cwd()) { await fs.writeFile(target, openSection(changelog, version)); return; } + const linked = fragments.map(fragment => + linkFragment(fragment, pullRequestOf), + ); const tagged = - fragments.length > 0 && + linked.length > 0 && isFinalized(changelog, version) && releaseTagExists(repository.url, version); await fs.writeFile( target, - prepareRelease(changelog, version, fragments, { tagged }), + prepareRelease(changelog, version, linked, { tagged }), ); for (const fragment of fragments) { await fs.unlink(path.join(root, "changes", fragment.name)); diff --git a/scripts/changelog.test.js b/scripts/changelog.test.js index 28a4838358..3b63c65b04 100644 --- a/scripts/changelog.test.js +++ b/scripts/changelog.test.js @@ -1,11 +1,14 @@ import assert from "node:assert/strict"; +import { execFileSync } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import test from "node:test"; import { + addedByPullRequest, categories, checkPullRequest, + linkFragment, openSection, parseFragment, placeholderLink, @@ -26,8 +29,8 @@ test("validates category names and bullet content", () => { ["recovery.bugfix.md", note], ["recovery.md", note], ["recovery.fix.md", ""], - ["recovery.fix.md", "- Missing link"], ["recovery.fix.md", `Heading\n${note}`], + ["recovery.fix.md", "- Bad\ncontinuation"], ]) { assert.throws(() => parseFragment(name, content)); } @@ -217,6 +220,106 @@ test("validates before mutation, sorts fragments, consumes only on release", asy await assert.rejects(run("release", root)); }); +test("links bullets without a link to the pull request that added them", () => { + const fragment = parseFragment( + "recovery.fix.md", + "- Fix recovery\n after errors.\n- Fix formatting. https://github.com/rescript-lang/rescript/pull/7", + ); + assert.equal(fragment.unlinked, true); + assert.equal(fragment.pr, 7); + const linked = linkFragment(fragment, () => 42); + assert.equal( + linked.text, + "- Fix recovery\n after errors. https://github.com/rescript-lang/rescript/pull/42\n- Fix formatting. https://github.com/rescript-lang/rescript/pull/7", + ); + assert.equal(linked.pr, 42); + const plain = parseFragment("plain.fix.md", "- Fix recovery."); + assert.equal(plain.pr, null); + assert.equal( + linkFragment(parseFragment("linked.fix.md", note), () => { + throw new Error("not looked up"); + }).text, + note, + ); + assert.throws( + () => linkFragment(plain, () => null), + /changes\/plain\.fix\.md: no PR link/, + ); +}); + +test("orders fragments without a link first in previews", () => { + const linked = parseFragment("a.fix.md", note); + const unlinked = parseFragment("b.fix.md", "- Not merged yet."); + assert.ok( + renderFragments([linked, unlinked]).includes(`- Not merged yet.\n${note}`), + ); +}); + +test("releases fragments without a link with their pull request's link", async t => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "rescript-changelog-")); + t.after(() => fs.rm(root, { recursive: true, force: true })); + await fs.mkdir(path.join(root, "changes")); + await fs.writeFile( + path.join(root, "package.json"), + JSON.stringify({ version: "13.0.0" }), + ); + await fs.writeFile(path.join(root, "CHANGELOG.md"), changelog); + await fs.writeFile(path.join(root, "changes", "old.fix.md"), note); + await fs.writeFile(path.join(root, "changes", "new.fix.md"), "- New fix."); + await fs.writeFile(path.join(root, "changes", "lost.fix.md"), "- Lost."); + const numbers = { "new.fix.md": 9000, "lost.fix.md": null }; + await assert.rejects( + run("release", root, name => numbers[name]), + /changes\/lost\.fix\.md/, + ); + assert.equal( + await fs.readFile(path.join(root, "CHANGELOG.md"), "utf8"), + changelog, + ); + await fs.unlink(path.join(root, "changes", "lost.fix.md")); + await run("release", root, name => numbers[name]); + assert.ok( + (await fs.readFile(path.join(root, "CHANGELOG.md"), "utf8")).includes( + `${categories.fix}\n\n- New fix. https://github.com/rescript-lang/rescript/pull/9000\n${note}\n`, + ), + ); +}); + +test("finds the pull request in the subject of the commit that added a fragment", async t => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "rescript-changelog-")); + t.after(() => fs.rm(root, { recursive: true, force: true })); + const git = (...args) => + execFileSync("git", args, { + cwd: root, + env: { + ...process.env, + GIT_AUTHOR_NAME: "a", + GIT_AUTHOR_EMAIL: "a@example.com", + GIT_COMMITTER_NAME: "a", + GIT_COMMITTER_EMAIL: "a@example.com", + }, + }); + git("init", "-q"); + await fs.mkdir(path.join(root, "changes")); + const commit = async (name, subject) => { + await fs.writeFile(path.join(root, "changes", name), "- Note."); + git("add", "."); + git("commit", "-q", "-m", subject); + }; + await commit("squashed.fix.md", "Fix recovery (#123)"); + await commit("rebased.fix.md", "Fix formatting"); + assert.equal(addedByPullRequest(root, "squashed.fix.md"), 123); + assert.equal(addedByPullRequest(root, "rebased.fix.md"), null); + assert.equal(addedByPullRequest(root, "missing.fix.md"), null); + // A later edit by another pull request does not change who added it. + await fs.writeFile( + path.join(root, "changes", "squashed.fix.md"), + "- Edited.", + ); + git("commit", "-q", "-am", "Reword (#456)"); + assert.equal(addedByPullRequest(root, "squashed.fix.md"), 123); +}); + test("preview groups categories without consuming fragments", () => { const fragments = [parseFragment("recovery.fix.md", note)]; const preview = renderFragments(fragments); From 73d89ca3d541511f752a36101886ab9d663ac050 Mon Sep 17 00:00:00 2001 From: Christoph Knittel Date: Sun, 18 Oct 2026 06:43:04 +0000 Subject: [PATCH 2/2] Preview merged fragments with their derived links Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01NP47qMfjMbjUTKZDsLthDo Signed-off-by: Christoph Knittel --- scripts/changelog.js | 14 +++++++++++++- scripts/changelog.test.js | 24 ++++++++++++++++++++---- 2 files changed, 33 insertions(+), 5 deletions(-) diff --git a/scripts/changelog.js b/scripts/changelog.js index ee15c8045f..33e814f393 100644 --- a/scripts/changelog.js +++ b/scripts/changelog.js @@ -100,6 +100,14 @@ export function linkFragment(fragment, pullRequestOf) { }; } +// What the release will write for a fragment: linked once its pull request is +// merged, and as written before that. +export function previewFragment(fragment, pullRequestOf) { + if (!fragment.unlinked) return fragment; + const pr = pullRequestOf(fragment.name); + return pr ? linkFragment(fragment, () => pr) : fragment; +} + export async function readFragments(root) { const directory = path.join(root, "changes"); const entries = await fs.readdir(directory, { withFileTypes: true }); @@ -308,7 +316,11 @@ export async function run( return; } if (command === "preview") { - process.stdout.write(renderFragments(fragments)); + process.stdout.write( + renderFragments( + fragments.map(fragment => previewFragment(fragment, pullRequestOf)), + ), + ); return; } const { version, repository } = JSON.parse( diff --git a/scripts/changelog.test.js b/scripts/changelog.test.js index 3b63c65b04..0a4ed21d67 100644 --- a/scripts/changelog.test.js +++ b/scripts/changelog.test.js @@ -13,6 +13,7 @@ import { parseFragment, placeholderLink, prepareRelease, + previewFragment, readFragments, renderFragments, run, @@ -247,11 +248,26 @@ test("links bullets without a link to the pull request that added them", () => { ); }); -test("orders fragments without a link first in previews", () => { - const linked = parseFragment("a.fix.md", note); - const unlinked = parseFragment("b.fix.md", "- Not merged yet."); +test("previews merged fragments with their links, newest PR first", () => { + const numbers = { "a.fix.md": 10, "b.fix.md": 30, "c.fix.md": null }; + const fragments = [ + parseFragment("a.fix.md", "- Older."), + parseFragment("b.fix.md", "- Newer."), + parseFragment("c.fix.md", "- Not merged yet."), + parseFragment( + "d.fix.md", + "- Linked. https://github.com/rescript-lang/rescript/pull/20", + ), + ].map(fragment => previewFragment(fragment, name => numbers[name])); assert.ok( - renderFragments([linked, unlinked]).includes(`- Not merged yet.\n${note}`), + renderFragments(fragments).includes( + [ + "- Not merged yet.", + "- Newer. https://github.com/rescript-lang/rescript/pull/30", + "- Linked. https://github.com/rescript-lang/rescript/pull/20", + "- Older. https://github.com/rescript-lang/rescript/pull/10", + ].join("\n"), + ), ); });