From b8cf8032cde284cf56efb227b578c9bb16971fea Mon Sep 17 00:00:00 2001 From: Luke Karrys Date: Mon, 15 Apr 2024 13:19:45 -0700 Subject: [PATCH] fix: only use latest template-oss specific commits in changelog (#430) --- lib/release/changelog.js | 67 +++++++++++++++++++++++++++++++-------- test/release/changelog.js | 59 ++++++++++++++++++++++++++++++---- 2 files changed, 107 insertions(+), 19 deletions(-) diff --git a/lib/release/changelog.js b/lib/release/changelog.js index 14450096..355edbb7 100644 --- a/lib/release/changelog.js +++ b/lib/release/changelog.js @@ -110,7 +110,7 @@ class ChangelogNotes { return authorsByCommit } - async #getPullRequestForCommits (commits) { + async #getPullRequestNumbersForCommits (commits) { const shas = commits .filter(c => !c.pullRequest?.number) .map(c => c.sha) @@ -134,7 +134,7 @@ class ChangelogNotes { return pullRequestsByCommit } - #buildEntry (commit, { authors = [], pullRequest }) { + #buildEntry (commit) { const entry = [] if (commit.sha) { @@ -143,7 +143,7 @@ class ChangelogNotes { } // A link to the pull request if the commit has one - const commitPullRequest = commit.pullRequest?.number ?? pullRequest + const commitPullRequest = commit.pullRequestNumber if (commitPullRequest) { entry.push(link(`#${commitPullRequest}`, this.#ghUrl('pull', commitPullRequest))) } @@ -154,21 +154,65 @@ class ChangelogNotes { entry.push([scope, subject].filter(Boolean).join(' ')) // A list og the authors github handles or names - if (authors.length) { - entry.push(`(${authors.join(', ')})`) + if (commit.authors.length) { + entry.push(`(${commit.authors.join(', ')})`) } return entry.join(' ') } - async buildNotes (commits, { version, previousTag, currentTag, changelogSections }) { + #filterCommits (commits) { + const filteredCommits = [] + const keyedDuplicates = {} + + // Filter certain commits so we can make sure only the latest version of + // each one gets into the changelog + for (const commit of commits) { + if (commit.bareMessage.startsWith('postinstall for dependabot template-oss PR')) { + keyedDuplicates.templateOssPostInstall ??= [] + keyedDuplicates.templateOssPostInstall.push(commit) + continue + } + + if (commit.bareMessage.startsWith('bump @npmcli/template-oss from')) { + keyedDuplicates.templateOssBump ??= [] + keyedDuplicates.templateOssBump.push(commit) + continue + } + + filteredCommits.push(commit) + } + + // Sort all our duplicates so we get the latest verion (by PR number) of each type. + // Then flatten so we can put them all back into the changelog + const sortedDupes = Object.values(keyedDuplicates) + .filter((items) => Boolean(items.length)) + .map((items) => items.sort((a, b) => b.pullRequestNumber - a.pullRequestNumber)) + .flatMap(items => items[0]) + + // This moves them to the bottom of their changelog section which is not + // strictly necessary but it's easier to do this way. + for (const duplicate of sortedDupes) { + filteredCommits.push(duplicate) + } + + return filteredCommits + } + + async buildNotes (rawCommits, { version, previousTag, currentTag, changelogSections }) { // get authors for commits for each sha - const authorsByCommit = await this.#getAuthorsForCommits(commits) + const authors = await this.#getAuthorsForCommits(rawCommits) // when rebase merging multiple commits with a single PR, only the first commit // will have a pr number when coming from release-please. this check will manually // lookup commits without a pr number and find one if it exists - const pullRequestByCommit = await this.#getPullRequestForCommits(commits) + const prNumbers = await this.#getPullRequestNumbersForCommits(rawCommits) + + const fullCommits = rawCommits.map((commit) => { + commit.authors = authors[commit.sha] ?? [] + commit.pullRequestNumber = Number(commit.pullRequest?.number ?? prNumbers[commit.sha]) + return commit + }) const changelog = new Changelog({ version, @@ -178,12 +222,9 @@ class ChangelogNotes { sections: changelogSections, }) - for (const commit of commits) { + for (const commit of this.#filterCommits(fullCommits)) { // Collect commits by type - changelog.add(commit.type, this.#buildEntry(commit, { - authors: authorsByCommit[commit.sha], - pullRequest: pullRequestByCommit[commit.sha], - })) + changelog.add(commit.type, this.#buildEntry(commit)) // And breaking changes to its own section changelog.add(Changelog.BREAKING, ...commit.notes diff --git a/test/release/changelog.js b/test/release/changelog.js index 10a3160a..838b0d74 100644 --- a/test/release/changelog.js +++ b/test/release/changelog.js @@ -36,11 +36,13 @@ const mockGitHub = ({ commits, authors }) => ({ }, }) -const mockChangelog = async ({ shas = true, authors = true, previousTag = true } = {}) => { - const commits = [{ +const mockChangelog = async ({ + shas = true, + authors = true, + previousTag = true, + commits: rawCommits = [{ sha: 'a', type: 'feat', - notes: [], bareMessage: 'Hey now', scope: 'bin', }, { @@ -55,13 +57,15 @@ const mockChangelog = async ({ shas = true, authors = true, previousTag = true } sha: 'c', type: 'deps', bareMessage: 'test@1.2.3', - notes: [], }, { sha: 'd', type: 'fix', bareMessage: 'this fixes it', - notes: [], - }].map(({ sha, ...rest }) => shas ? { sha, ...rest } : rest) + }], +} = {}) => { + const commits = rawCommits + .map(({ notes = [], ...rest }) => ({ notes, ...rest })) + .map(({ sha, ...rest }) => shas ? { sha, ...rest } : { ...rest }) const github = mockGitHub({ commits, authors }) const changelog = new ChangelogNotes(github) @@ -112,3 +116,46 @@ t.test('no tag/authors/shas', async t => { '* `test@1.2.3`', ]) }) + +t.test('filters out multiple template oss commits', async t => { + const changelog = await mockChangelog({ + authors: false, + commits: [{ + sha: 'a', + type: 'chore', + bareMessage: 'postinstall for dependabot template-oss PR', + pullRequest: { + number: '100', + }, + }, { + sha: 'b', + type: 'chore', + bareMessage: 'postinstall for dependabot template-oss PR', + pullRequest: { + number: '101', + }, + }, { + sha: 'c', + type: 'chore', + bareMessage: 'bump @npmcli/template-oss from 1 to 2', + pullRequest: { + number: '101', + }, + }, { + sha: 'd', + type: 'chore', + bareMessage: 'bump @npmcli/template-oss from 0 to 1', + pullRequest: { + number: '100', + }, + }], + }) + t.strictSame(changelog, [ + '## [1.0.0](https://github.com/npm/cli/compare/v0.1.0...v1.0.0) (DATE)', + '### Chores', + // eslint-disable-next-line max-len + '* [`b`](https://github.com/npm/cli/commit/b) [#101](https://github.com/npm/cli/pull/101) postinstall for dependabot template-oss PR', + // eslint-disable-next-line max-len + '* [`c`](https://github.com/npm/cli/commit/c) [#101](https://github.com/npm/cli/pull/101) bump @npmcli/template-oss from 1 to 2', + ]) +})