Skip to content

Commit 5ebfe97

Browse files
authored
Fix bug introduced to updateChangelog in #158 (#181)
## Motivation #158 incorrectly refactored the branches in updateChangelog, resulting in a state where new changelog entries weren't added to the return value if isReleaseCandidate is false. ## Explanation - This error is fixed by moving the logic for retrieving new entries out of the if (isReleaseCandidate) block. - The code for retrieving new entries is also extracted into its own method: getNewChangeEntries, abstracting away details that obscured the flow of the method's core logic. - To verify that the buggy logic is correctly restored, it's necessary to compare the current state of updateChangelog to its state before the bug was introduced. For this, see: [diff link](https://github.com/MetaMask/auto-changelog/compare/e8df1ec717f534c8fe84c46ea86a847fa5a32973..da39a58a55571cf4e8a03e28096aa12c02c75f77#diff-4228e8302e41dd1c51e813af8368efdcb40b3d9db3e3f21f417ae87275d9f389R249-R316). ## References - Closes #180 - Followed by #188
1 parent bb8b47a commit 5ebfe97

File tree

1 file changed

+71
-44
lines changed

1 file changed

+71
-44
lines changed

src/update-changelog.ts

Lines changed: 71 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { strict as assert } from 'assert';
22
import execa from 'execa';
33

44
import type Changelog from './changelog';
5-
import { Formatter } from './changelog';
5+
import { Formatter, getKnownPropertyNames } from './changelog';
66
import { ChangeCategory, Version } from './constants';
77
import { parseChangelog } from './parse-changelog';
88
import { PackageRename } from './shared-types';
@@ -20,6 +20,9 @@ async function getMostRecentTag({
2020
}: {
2121
tagPrefixes: [string, ...string[]];
2222
}) {
23+
// Ensure we have all tags on remote
24+
await runCommand('git', ['fetch', '--tags']);
25+
2326
let mostRecentTagCommitHash: string | null = null;
2427
for (const tagPrefix of tagPrefixes) {
2528
const revListArgs = [
@@ -157,6 +160,53 @@ async function getCommitHashesInRange(
157160
return await runCommand('git', revListArgs);
158161
}
159162

163+
type AddNewCommitsOptions = {
164+
mostRecentTag: string | null;
165+
repoUrl: string;
166+
loggedPrNumbers: string[];
167+
projectRootDirectory?: string;
168+
};
169+
170+
/**
171+
* Get the list of new change entries to add to a changelog.
172+
*
173+
* @param options - Options.
174+
* @param options.mostRecentTag - The most recent tag.
175+
* @param options.repoUrl - The GitHub repository URL for the current project.
176+
* @param options.loggedPrNumbers - A list of all pull request numbers included in the relevant parsed changelog.
177+
* @param options.projectRootDirectory - The root project directory, used to
178+
* filter results from various git commands. This path is assumed to be either
179+
* absolute, or relative to the current directory. Defaults to the root of the
180+
* current git repository.
181+
* @returns A list of new change entries to add to the changelog, based on commits made since the last release.
182+
*/
183+
async function getNewChangeEntries({
184+
mostRecentTag,
185+
repoUrl,
186+
loggedPrNumbers,
187+
projectRootDirectory,
188+
}: AddNewCommitsOptions) {
189+
const commitRange =
190+
mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
191+
const commitsHashesSinceLastRelease = await getCommitHashesInRange(
192+
commitRange,
193+
projectRootDirectory,
194+
);
195+
const commits = await getCommits(commitsHashesSinceLastRelease);
196+
197+
const newCommits = commits.filter(
198+
({ prNumber }) => !prNumber || !loggedPrNumbers.includes(prNumber),
199+
);
200+
201+
return newCommits.map(({ prNumber, description }) => {
202+
if (prNumber) {
203+
const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
204+
return `${description} ${suffix}`;
205+
}
206+
return description;
207+
});
208+
}
209+
160210
export type UpdateChangelogOptions = {
161211
changelogContent: string;
162212
currentVersion?: Version;
@@ -213,43 +263,17 @@ export async function updateChangelog({
213263
packageRename,
214264
});
215265

216-
// Ensure we have all tags on remote
217-
await runCommand('git', ['fetch', '--tags']);
218266
const mostRecentTag = await getMostRecentTag({
219267
tagPrefixes,
220268
});
221269

222-
const commitRange =
223-
mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
224-
const commitsHashesSinceLastRelease = await getCommitHashesInRange(
225-
commitRange,
226-
projectRootDirectory,
227-
);
228-
const commits = await getCommits(commitsHashesSinceLastRelease);
229-
230-
const loggedPrNumbers = getAllLoggedPrNumbers(changelog);
231-
const newCommits = commits.filter(
232-
({ prNumber }) =>
233-
prNumber === undefined || !loggedPrNumbers.includes(prNumber),
234-
);
235-
236-
const hasUnreleasedChanges =
237-
Object.keys(changelog.getUnreleasedChanges()).length !== 0;
238-
if (
239-
newCommits.length === 0 &&
240-
(!isReleaseCandidate || hasUnreleasedChanges)
241-
) {
242-
return undefined;
243-
}
244-
245270
if (isReleaseCandidate) {
246271
if (!currentVersion) {
247272
throw new Error(
248273
`A version must be specified if 'isReleaseCandidate' is set.`,
249274
);
250275
}
251-
252-
if (mostRecentTag === `${tagPrefixes[0]}${currentVersion ?? ''}`) {
276+
if (mostRecentTag === `${tagPrefixes[0]}${currentVersion}`) {
253277
throw new Error(
254278
`Current version already has a tag ('${mostRecentTag}'), which is unexpected for a release candidate.`,
255279
);
@@ -264,28 +288,31 @@ export async function updateChangelog({
264288
changelog.addRelease({ version: currentVersion });
265289
}
266290

267-
if (hasUnreleasedChanges) {
291+
const hasUnreleasedChangesToRelease =
292+
getKnownPropertyNames(changelog.getUnreleasedChanges()).length > 0;
293+
if (hasUnreleasedChangesToRelease) {
268294
changelog.migrateUnreleasedChangesToRelease(currentVersion);
269295
}
296+
}
270297

271-
const newChangeEntries = newCommits.map(({ prNumber, description }) => {
272-
if (prNumber) {
273-
const suffix = `([#${prNumber}](${repoUrl}/pull/${prNumber}))`;
274-
return `${description} ${suffix}`;
275-
}
276-
return description;
277-
});
298+
const newChangeEntries = await getNewChangeEntries({
299+
mostRecentTag,
300+
repoUrl,
301+
loggedPrNumbers: getAllLoggedPrNumbers(changelog),
302+
projectRootDirectory,
303+
});
278304

279-
for (const description of newChangeEntries.reverse()) {
280-
changelog.addChange({
281-
version: isReleaseCandidate ? currentVersion : undefined,
282-
category: ChangeCategory.Uncategorized,
283-
description,
284-
});
285-
}
305+
for (const description of newChangeEntries.reverse()) {
306+
changelog.addChange({
307+
version: isReleaseCandidate ? currentVersion : undefined,
308+
category: ChangeCategory.Uncategorized,
309+
description,
310+
});
286311
}
287312

288-
return changelog.toString();
313+
const newChangelogContent = changelog.toString();
314+
const isChangelogUpdated = changelogContent !== newChangelogContent;
315+
return isChangelogUpdated ? newChangelogContent : undefined;
289316
}
290317

291318
/**

0 commit comments

Comments
 (0)