diff --git a/scripts/update-changelog.ts b/scripts/update-changelog.ts index b418abf..cb8c46e 100644 --- a/scripts/update-changelog.ts +++ b/scripts/update-changelog.ts @@ -47,6 +47,8 @@ export interface Commit { isBreaking: boolean author: { name: string, email: string } references: string[] + /** Hash of the commit this one reverts, when its body names one. */ + revert?: string } interface Contributor { @@ -68,6 +70,7 @@ const TYPE_TITLES: Record = { test: '✅ Tests', style: '🎨 Styles', ci: '🤖 CI', + revert: '⏪ Reverts', } const KNOWN_TYPES = new Set(Object.keys(TYPE_TITLES)) @@ -181,21 +184,28 @@ function parseCommit (raw: string): Commit | null { const [hash, shortHash, authorName, authorEmail, subject, body] = raw.split('\x1f') if (!hash || !shortHash || !subject) return null - const header = subject.match(/^(\w+)(?:\(([^)]+)\))?(!)?:\s*(.+)$/) + const revert = (body || '').match(/\breverts\s+(?:commit\s+)?([0-9a-f]{7,40})\b/i)?.[1]?.toLowerCase() + + // `git revert` and GitLab generate `Revert ""`; treat it + // as a `revert:` commit carrying the original's scope and description. + const reverted = subject.match(/^Revert "(.+)"$/)?.[1] + const header = (reverted ?? subject).match(/^(\w+)(?:\(([^)]+)\))?(!)?:\s*(.+)$/) if (!header) { return { hash, shortHash, message: subject, - type: '', + type: reverted ? 'revert' : '', scope: '', - description: subject, + description: reverted ?? subject, isBreaking: false, author: { name: authorName || '', email: authorEmail || '' }, references: [], + revert, } } - const [, type, scope = '', bang, rawDescription] = header + const [, parsedType, scope = '', bang, rawDescription] = header + const type = reverted ? 'revert' : parsedType!.toLowerCase() const isBreaking = Boolean(bang) || /BREAKING[ -]CHANGE/.test(body || '') const references: string[] = [] @@ -213,15 +223,47 @@ function parseCommit (raw: string): Commit | null { hash, shortHash, message: subject, - type: type!.toLowerCase(), + type, scope, description, isBreaking, author: { name: authorName || '', email: authorEmail || '' }, references: [...new Set(references)], + revert, } } +/** + * Collapse revert pairs: a revert whose target is in the same range drops + * out along with the target, since together they change nothing. A chain of + * reverts resolves to the commit it ultimately targets, which survives when + * an even number of reverts leaves it applied. A revert whose target shipped + * earlier is kept and rendered as an ordinary entry. + */ +export function dropRevertedCommits (commits: Commit[]): Commit[] { + const find = (prefix: string) => commits.find(c => c.hash.startsWith(prefix)) + const paired = new Set() + const revertCount = new Map() + for (const commit of commits) { + if (commit.type !== 'revert' || !commit.revert) continue + let root = find(commit.revert) + if (!root) continue + const seen = new Set([commit.hash]) + while (root.type === 'revert' && root.revert && !seen.has(root.hash)) { + seen.add(root.hash) + const next = find(root.revert) + if (!next) break + root = next + } + paired.add(commit.hash) + revertCount.set(root.hash, (revertCount.get(root.hash) ?? 0) + 1) + } + if (!paired.size) return commits + return commits.filter(c => + !paired.has(c.hash) && (revertCount.get(c.hash) ?? 0) % 2 === 0, + ) +} + /** * Drop commits whose subject already appears on the other side of a * diverged range. The previous release tag is not always an ancestor of @@ -272,7 +314,7 @@ function getCommitsSince (tag: Tag | null): Commit[] { .map(parseCommit) .filter((c): c is Commit => c !== null) - return tag ? dropAlreadyReleased(commits, subjectsOnlyOn(tag.ref)) : commits + return dropRevertedCommits(tag ? dropAlreadyReleased(commits, subjectsOnlyOn(tag.ref)) : commits) } export function determineBump (commits: Commit[]): BumpLevel { diff --git a/test/update-changelog-main.test.ts b/test/update-changelog-main.test.ts index 41e8fc5..92e7a58 100644 --- a/test/update-changelog-main.test.ts +++ b/test/update-changelog-main.test.ts @@ -521,6 +521,64 @@ describe('lockstep main', () => { expect(prBody()).not.toContain('not a conventional commit') }) + it('releases a revert of an already-released commit', async () => { + git.commits = [{ ...FEAT, subject: 'revert: fix: the thing', body: 'This commit reverts 1234567' }] + await main() + expect(prBody()).toContain('### ⏪ Reverts') + expect(prBody()).toContain('- fix: the thing (') + const created = calls.find(c => c.method === 'POST' && c.path.endsWith('/pulls'))! + expect(created.body).toMatchObject({ title: 'v1.2.4' }) + }) + + it('releases the default `git revert` subject, keeping the original scope', async () => { + git.commits = [{ ...FEAT, subject: 'Revert "feat(api): add widgets"', body: 'This reverts commit 1234567.' }] + await main() + expect(prBody()).toContain('### ⏪ Reverts') + expect(prBody()).toContain('**api:** add widgets') + }) + + it('releases a patch when a breaking change is reverted before it ships', async () => { + const breaking: FakeCommit = { hash: 'b'.repeat(40), short: 'bbbbbbb', name: 'Bo', email: 'bo@example.com', subject: 'feat!: remove the old API' } + git.commits = [ + { ...FEAT, hash: 'c'.repeat(40), short: 'ccccccc', subject: `Revert "${breaking.subject}"`, body: `This reverts commit ${breaking.hash}.` }, + breaking, + { ...FEAT, hash: 'd'.repeat(40), short: 'ddddddd', subject: 'fix: something else (#9)' }, + ] + git.revList = [breaking.hash] + api.logins = new Map([['ccccccc', 'ada'], ['bbbbbbb', 'bo'], ['ddddddd', 'ada']]) + await main() + const created = calls.find(c => c.method === 'POST' && c.path.endsWith('/pulls'))! + expect(created.body).toMatchObject({ title: 'v1.2.4' }) + expect(prBody()).not.toContain('remove the old API') + }) + + it('releases a major when reverting a breaking change from an earlier release', async () => { + git.commits = [{ ...FEAT, subject: 'Revert "feat!: remove the old API"', body: 'This reverts commit 1234567.' }] + await main() + const created = calls.find(c => c.method === 'POST' && c.path.endsWith('/pulls'))! + expect(created.body).toMatchObject({ title: 'v2.0.0' }) + }) + + it('releases a revert of a non-conventional subject', async () => { + git.commits = [{ ...FEAT, subject: 'Revert "tidy up the thing"', body: 'This reverts commit 1234567.' }] + await main() + expect(prBody()).toContain('- tidy up the thing (') + }) + + it('drops a commit and its revert when both land in the same release', async () => { + const reverted: FakeCommit = { hash: 'b'.repeat(40), short: 'bbbbbbb', name: 'Bo', email: 'bo@example.com', subject: 'fix: temporary (#8)' } + git.commits = [ + { ...FEAT, hash: 'c'.repeat(40), short: 'ccccccc', subject: 'revert: fix: temporary', body: `This reverts commit ${reverted.hash}.` }, + reverted, + FEAT, + ] + git.revList = [FEAT.hash, reverted.hash] + api.logins = new Map([['ccccccc', 'ada'], ['bbbbbbb', 'bo'], ['aaaaaaa', 'ada']]) + await main() + expect(prBody()).toContain('- add a thing (#7)') + expect(prBody()).not.toContain('temporary') + }) + it('collects issue references from the commit body', async () => { git.commits = [{ ...FEAT, subject: 'fix: repair the thing', body: 'Fixes #12\nBREAKING CHANGE: it moved' }] await main() diff --git a/test/update-changelog.test.ts b/test/update-changelog.test.ts index af4cc19..5874a31 100644 --- a/test/update-changelog.test.ts +++ b/test/update-changelog.test.ts @@ -3,7 +3,7 @@ import { tmpdir } from 'node:os' import { resolve } from 'node:path' import { afterEach, beforeEach, describe, expect, it } from 'vitest' -import { stripPlaceholderTimetable, TIMETABLE_PLACEHOLDER, buildBumpFileSet, buildIndependentBody, determineBump, formatChangelog, buildIndependentBumpFileSet, computeIndependentPlan, extractPreamble, truncateBody, dropAlreadyReleased, incVersion, latestLockstepTag, latestTagForPackage, releaseBranchDrift, isSupersededReleaseBranch, type Commit } from '../scripts/update-changelog.ts' +import { stripPlaceholderTimetable, TIMETABLE_PLACEHOLDER, buildBumpFileSet, dropRevertedCommits, buildIndependentBody, determineBump, formatChangelog, buildIndependentBumpFileSet, computeIndependentPlan, extractPreamble, truncateBody, dropAlreadyReleased, incVersion, latestLockstepTag, latestTagForPackage, releaseBranchDrift, isSupersededReleaseBranch, type Commit } from '../scripts/update-changelog.ts' import { resolveWorkspaces } from '../scripts/_workspaces.ts' let tmp: string @@ -551,6 +551,68 @@ describe('dropAlreadyReleased', () => { }) }) +describe('dropRevertedCommits', () => { + const commit = (hash: string, type: string, revert?: string): Commit => ({ + hash, + shortHash: hash.slice(0, 7), + message: `${type}: ${hash}`, + type, + scope: '', + description: hash, + isBreaking: false, + author: { name: 'a', email: 'a@b.c' }, + references: [], + revert, + }) + + it('drops a revert and the commit it reverts', () => { + const commits = [commit('b'.repeat(40), 'revert', 'a'.repeat(7)), commit('a'.repeat(40), 'fix')] + expect(dropRevertedCommits(commits)).toEqual([]) + }) + + it('keeps a revert whose target is outside the range', () => { + const commits = [commit('b'.repeat(40), 'revert', 'c'.repeat(7)), commit('a'.repeat(40), 'fix')] + expect(dropRevertedCommits(commits)).toBe(commits) + }) + + it('keeps a revert commit with no parseable target', () => { + const commits = [commit('b'.repeat(40), 'revert')] + expect(dropRevertedCommits(commits)).toBe(commits) + }) + + it('restores a commit whose revert is itself reverted', () => { + const commits = [ + commit('c'.repeat(40), 'revert', 'b'.repeat(7)), + commit('b'.repeat(40), 'revert', 'a'.repeat(7)), + commit('a'.repeat(40), 'fix'), + ] + expect(dropRevertedCommits(commits).map(c => c.hash)).toEqual(['a'.repeat(40)]) + }) + + it('resolves a chain whose root shipped in an earlier release', () => { + const commits = [ + commit('c'.repeat(40), 'revert', 'b'.repeat(7)), + commit('b'.repeat(40), 'revert', 'f'.repeat(7)), + ] + expect(dropRevertedCommits(commits)).toEqual([]) + }) + + it('ignores a revert that names itself', () => { + const commits = [commit('b'.repeat(40), 'revert', 'b'.repeat(7))] + expect(dropRevertedCommits(commits)).toEqual([]) + }) + + it('drops an even-length revert chain entirely', () => { + const commits = [ + commit('d'.repeat(40), 'revert', 'c'.repeat(7)), + commit('c'.repeat(40), 'revert', 'b'.repeat(7)), + commit('b'.repeat(40), 'revert', 'a'.repeat(7)), + commit('a'.repeat(40), 'fix'), + ] + expect(dropRevertedCommits(commits)).toEqual([]) + }) +}) + describe('truncateBody', () => { it('leaves bodies within the limit untouched', () => { expect(truncateBody('short body')).toBe('short body')