Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 54 additions & 13 deletions lib/release/changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -134,7 +134,7 @@ class ChangelogNotes {
return pullRequestsByCommit
}

#buildEntry (commit, { authors = [], pullRequest }) {
#buildEntry (commit) {
const entry = []

if (commit.sha) {
Expand All @@ -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)))
}
Expand All @@ -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,
Expand All @@ -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
Expand Down
59 changes: 53 additions & 6 deletions test/release/changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}, {
Expand All @@ -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)
Expand Down Expand Up @@ -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',
])
})
Loading