From 3008846cede8b58eed1ad3afbc4c4e8b66026e74 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Mon, 11 Dec 2023 09:05:30 -0800 Subject: [PATCH 1/6] [update-changelog] Fix bug introduced in #158 - `updateChangelog` returns results even if `isReleaseCandidate` is false --- src/update-changelog.ts | 43 ++++++++++++++++++++--------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index 912fb21..490d8d2 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -205,6 +205,11 @@ export async function updateChangelog({ formatter = undefined, packageRename, }: UpdateChangelogOptions): Promise { + if (isReleaseCandidate && !currentVersion) { + throw new Error( + `A version must be specified if 'isReleaseCandidate' is set.`, + ); + } const changelog = parseChangelog({ changelogContent, repoUrl, @@ -242,14 +247,8 @@ export async function updateChangelog({ return undefined; } - if (isReleaseCandidate) { - if (!currentVersion) { - throw new Error( - `A version must be specified if 'isReleaseCandidate' is set.`, - ); - } - - if (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) { + if (isReleaseCandidate && currentVersion) { + if (mostRecentTag === `${tagPrefixes[0]}${currentVersion}`) { throw new Error( `Current version already has a tag ('${mostRecentTag}'), which is unexpected for a release candidate.`, ); @@ -267,22 +266,22 @@ export async function updateChangelog({ if (hasUnreleasedChanges) { changelog.migrateUnreleasedChangesToRelease(currentVersion); } + } - const newChangeEntries = newCommits.map(({ prNumber, description }) => { - if (prNumber) { - const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`; - return `${description} ${suffix}`; - } - return description; - }); - - for (const description of newChangeEntries.reverse()) { - changelog.addChange({ - version: isReleaseCandidate ? currentVersion : undefined, - category: ChangeCategory.Uncategorized, - description, - }); + const newChangeEntries = newCommits.map(({ prNumber, description }) => { + if (prNumber) { + const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`; + return `${description} ${suffix}`; } + return description; + }); + + for (const description of newChangeEntries.reverse()) { + changelog.addChange({ + version: isReleaseCandidate ? currentVersion : undefined, + category: ChangeCategory.Uncategorized, + description, + }); } return changelog.toString(); From 8cec96efa53dd1f180a99800adaf5caf82e74aa4 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 14 Dec 2023 13:39:36 -0800 Subject: [PATCH 2/6] [update-changelog] refactor: extract `getNewChangeEntries` method --- src/update-changelog.ts | 103 +++++++++++++++++++++++++--------------- 1 file changed, 66 insertions(+), 37 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index 490d8d2..76daf2d 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -157,6 +157,54 @@ async function getCommitHashesInRange( return await runCommand('git', revListArgs); } +type AddNewCommitsOptions = { + mostRecentTag: string | null; + repoUrl: string; + loggedPrNumbers: string[]; + projectRootDirectory?: string; +}; + +/** + * Get the list of new change entries to add to a changelog. + * + * @param options - Options. + * @param options.mostRecentTag - The most recent tag. + * @param options.repoUrl - The GitHub repository URL for the current project. + * @param options.loggedPrNumbers - A list of all pull request numbers included in the relevant parsed changelog. + * @param options.projectRootDirectory - The root project directory, used to + * filter results from various git commands. This path is assumed to be either + * absolute, or relative to the current directory. Defaults to the root of the + * current git repository. + * @returns A list of new change entries to add to the changelog, based on commits made since the last release. + */ +async function getNewChangeEntries({ + mostRecentTag, + repoUrl, + loggedPrNumbers, + projectRootDirectory, +}: AddNewCommitsOptions) { + const commitRange = + mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`; + const commitsHashesSinceLastRelease = await getCommitHashesInRange( + commitRange, + projectRootDirectory, + ); + const commits = await getCommits(commitsHashesSinceLastRelease); + + const newCommits = commits.filter( + ({ prNumber }) => + prNumber === undefined || !loggedPrNumbers.includes(prNumber), + ); + + return newCommits.map(({ prNumber, description }) => { + if (prNumber) { + const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`; + return `${description} ${suffix}`; + } + return description; + }); +} + export type UpdateChangelogOptions = { changelogContent: string; currentVersion?: Version; @@ -205,11 +253,6 @@ export async function updateChangelog({ formatter = undefined, packageRename, }: UpdateChangelogOptions): Promise { - if (isReleaseCandidate && !currentVersion) { - throw new Error( - `A version must be specified if 'isReleaseCandidate' is set.`, - ); - } const changelog = parseChangelog({ changelogContent, repoUrl, @@ -224,30 +267,12 @@ export async function updateChangelog({ tagPrefixes, }); - const commitRange = - mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`; - const commitsHashesSinceLastRelease = await getCommitHashesInRange( - commitRange, - projectRootDirectory, - ); - const commits = await getCommits(commitsHashesSinceLastRelease); - - const loggedPrNumbers = getAllLoggedPrNumbers(changelog); - const newCommits = commits.filter( - ({ prNumber }) => - prNumber === undefined || !loggedPrNumbers.includes(prNumber), - ); - - const hasUnreleasedChanges = - Object.keys(changelog.getUnreleasedChanges()).length !== 0; - if ( - newCommits.length === 0 && - (!isReleaseCandidate || hasUnreleasedChanges) - ) { - return undefined; - } - - if (isReleaseCandidate && currentVersion) { + if (isReleaseCandidate) { + if (!currentVersion) { + throw new Error( + `A version must be specified if 'isReleaseCandidate' is set.`, + ); + } if (mostRecentTag === `${tagPrefixes[0]}${currentVersion}`) { throw new Error( `Current version already has a tag ('${mostRecentTag}'), which is unexpected for a release candidate.`, @@ -263,17 +288,18 @@ export async function updateChangelog({ changelog.addRelease({ version: currentVersion }); } - if (hasUnreleasedChanges) { + const hasUnreleasedChangesToRelease = + Object.keys(changelog.getUnreleasedChanges()).length > 0; + if (hasUnreleasedChangesToRelease) { changelog.migrateUnreleasedChangesToRelease(currentVersion); } } - const newChangeEntries = newCommits.map(({ prNumber, description }) => { - if (prNumber) { - const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`; - return `${description} ${suffix}`; - } - return description; + const newChangeEntries = await getNewChangeEntries({ + mostRecentTag, + repoUrl, + loggedPrNumbers: getAllLoggedPrNumbers(changelog), + projectRootDirectory, }); for (const description of newChangeEntries.reverse()) { @@ -283,8 +309,11 @@ export async function updateChangelog({ description, }); } + const newChangelogContent = changelog.toString(); - return changelog.toString(); + return changelogContent === newChangelogContent + ? undefined + : newChangelogContent; } /** From d18d0fa42f288e4e29db1dc2e4e7ed37f0bf4148 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 8 Feb 2024 01:20:58 -0500 Subject: [PATCH 3/6] Move `git fetch --tags` call to `getMostRecentTag` --- src/update-changelog.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index 76daf2d..7ef39af 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -20,6 +20,9 @@ async function getMostRecentTag({ }: { tagPrefixes: [string, ...string[]]; }) { + // Ensure we have all tags on remote + await runCommand('git', ['fetch', '--tags']); + let mostRecentTagCommitHash: string | null = null; for (const tagPrefix of tagPrefixes) { const revListArgs = [ @@ -261,8 +264,6 @@ export async function updateChangelog({ packageRename, }); - // Ensure we have all tags on remote - await runCommand('git', ['fetch', '--tags']); const mostRecentTag = await getMostRecentTag({ tagPrefixes, }); From aeabb00d345641681a2ceb5988b4630d863ad7f7 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 8 Feb 2024 01:21:58 -0500 Subject: [PATCH 4/6] Rewrite return statement for readability --- src/update-changelog.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index 7ef39af..c24fc38 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -310,11 +310,10 @@ export async function updateChangelog({ description, }); } - const newChangelogContent = changelog.toString(); - return changelogContent === newChangelogContent - ? undefined - : newChangelogContent; + const newChangelogContent = changelog.toString(); + const isChangelogUpdated = changelogContent !== newChangelogContent; + return isChangelogUpdated ? newChangelogContent : undefined; } /** From da39a58a55571cf4e8a03e28096aa12c02c75f77 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 8 Feb 2024 01:43:14 -0500 Subject: [PATCH 5/6] Replace `Object.keys` with `getKnownPropertyNames` --- src/update-changelog.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index c24fc38..b3e6bbc 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -2,7 +2,7 @@ import { strict as assert } from 'assert'; import execa from 'execa'; import type Changelog from './changelog'; -import { Formatter } from './changelog'; +import { Formatter, getKnownPropertyNames } from './changelog'; import { ChangeCategory, Version } from './constants'; import { parseChangelog } from './parse-changelog'; import { PackageRename } from './shared-types'; @@ -290,7 +290,7 @@ export async function updateChangelog({ } const hasUnreleasedChangesToRelease = - Object.keys(changelog.getUnreleasedChanges()).length > 0; + getKnownPropertyNames(changelog.getUnreleasedChanges()).length > 0; if (hasUnreleasedChangesToRelease) { changelog.migrateUnreleasedChangesToRelease(currentVersion); } From 3aef0c90af16ef72c105bd6f4db9373d5cb734f3 Mon Sep 17 00:00:00 2001 From: Jongsun Suh Date: Thu, 8 Feb 2024 09:53:44 -0500 Subject: [PATCH 6/6] Apply suggestion to prefer negation over strict equals check for undefined --- src/update-changelog.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/update-changelog.ts b/src/update-changelog.ts index b3e6bbc..141dff8 100644 --- a/src/update-changelog.ts +++ b/src/update-changelog.ts @@ -195,8 +195,7 @@ async function getNewChangeEntries({ const commits = await getCommits(commitsHashesSinceLastRelease); const newCommits = commits.filter( - ({ prNumber }) => - prNumber === undefined || !loggedPrNumbers.includes(prNumber), + ({ prNumber }) => !prNumber || !loggedPrNumbers.includes(prNumber), ); return newCommits.map(({ prNumber, description }) => {