fix: only use latest template-oss specific commits in changelog (#430)

Author: lukekarrys

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

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: '[email protected]',
-    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 => {
     '* `[email protected]`',
   ])
 })
+
+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',
+  ])
+})