Skip to content

Commit ebe6c6e

Browse files
authored
feat(INFRA-3081): add --require-pr-numbers flag for generation-time filtering (#253)
## What is the current state of things and why does it need to change? Currently, `auto-changelog` includes all commits in the changelog, even direct commits without PR numbers. For projects following strict PR-based workflows (like MetaMask Extension), these direct commits should be excluded to ensure all changelog entries represent reviewed changes. ## What is the solution your changes offer and how does it work? ### 1. New `--requirePrNumbers` Flag Adds an opt-in CLI flag that filters out commits without PR numbers at generation time. Defaults to `false` for backward compatibility. **Files changed:** `cli.ts`, `update-changelog.ts`, `get-new-changes.ts` ### 2. Graceful 404 Handling for Forked Repos When running on forked repositories, the tool fetches PR details from the fork. PRs originating from the upstream repo return 404 errors. Added `try-catch` to return empty labels instead of crashing. ### 3. HTML Comment Stripping PR templates often contain HTML comments (`<!-- ... -->`) that were breaking markdown rendering in changelogs. Added `stripHtmlComments()` function to clean descriptions. ### 4. `CHANGELOG entry: null` Fix PRs marked with `CHANGELOG entry: null` were appearing as "Null (#PR)" due to: - Trailing whitespace not being trimmed - Case-sensitive comparison (only matched lowercase `null`) Fixed by adding `.trim()` and case-insensitive comparison at both check points. > **Note:** Fixes 2-4 were discovered during integration testing and are prerequisites for the `--requirePrNumbers` flag to work correctly in production environments with forked repos. ## Usage ```bash yarn auto-changelog update --requirePrNumbers --autoCategorize --useChangelogEntry --useShortPrLink --rc ``` ## Testing Tested with [consensys-test/metamask-extension-test](https://github.com/consensys-test/metamask-extension-test) on `release/1100.0.0`: | Test | Result | |------|--------| | PR Commit `feat: test commit with PR for 1100.0.0 (#40)` | ✅ **INCLUDED** | | Direct Commit `chore: direct commit without PR for 1100.0.0` | ✅ **EXCLUDED** | | `Null (#PR)` entries | ✅ **NONE** (case-insensitive fix working) | | [Generated Changelog](https://github.com/consensys-test/metamask-extension-test/blob/release/1100.0.0-Changelog/CHANGELOG.md) | ✅ Correct | ### Changelog Excerpt ```markdown ## [1100.0.0] ### Added - feat: test commit with PR for 1100.0.0 (#40) - Design upgrades for Confirmations (#38399) - Fix amount component to be only accept numeric inputs (#38235) ... ``` **Note:** `N/A` and `None` entries still appear (e.g., `N/A (#37810)`) because these are different strings from `null` - they're valid descriptions that PRs chose to use. ## References - Complements duplicate detection work: #251
1 parent e12cf19 commit ebe6c6e

File tree

4 files changed

+94
-14
lines changed

4 files changed

+94
-14
lines changed

README.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,14 @@ or
3838

3939
`yarn run auto-changelog update --useShortPrLink`
4040

41+
#### Require PR numbers (filter out commits without PR numbers)
42+
43+
- Only include commits that have associated PR numbers in the changelog
44+
- Commits without PR numbers (like direct commits) will be filtered out
45+
- This is useful for projects that want to ensure all changelog entries come from reviewed pull requests
46+
47+
`yarn run auto-changelog update --requirePrNumbers`
48+
4149
#### Update the current release section of the changelog
4250

4351
`yarn run auto-changelog update --rc`
@@ -50,6 +58,10 @@ or
5058

5159
`yarn run auto-changelog update --autoCategorize --useChangelogEntry --useShortPrLink --rc`
5260

61+
### With requirePrNumbers (for stricter PR-based workflows)
62+
63+
`yarn run auto-changelog update --autoCategorize --useChangelogEntry --useShortPrLink --requirePrNumbers --rc`
64+
5365
#### Update the changelog for a renamed package
5466

5567
This option is designed to be used for packages that live in a monorepo.
@@ -132,6 +144,7 @@ const updatedChangelog = await updateChangelog({
132144
currentVersion: '1.0.0',
133145
repoUrl: 'https://github.com/ExampleUsernameOrOrganization/ExampleRepository',
134146
isReleaseCandidate: false,
147+
requirePrNumbers: false, // Optional: set to true to filter out commits without PR numbers
135148
});
136149
await fs.writeFile('CHANGELOG.md', updatedChangelog);
137150
```

src/cli.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,10 @@ type UpdateOptions = {
102102
* Whether to use short PR links in the changelog entries
103103
*/
104104
useShortPrLink: boolean;
105+
/**
106+
* Whether to require PR numbers for all commits. If true, commits without PR numbers are filtered out.
107+
*/
108+
requirePrNumbers: boolean;
105109
};
106110

107111
/**
@@ -121,6 +125,7 @@ type UpdateOptions = {
121125
* @param options.autoCategorize - Whether to categorize commits automatically based on their messages.
122126
* @param options.useChangelogEntry - Whether to read `CHANGELOG entry:` from the commit body and the no-changelog label.
123127
* @param options.useShortPrLink - Whether to use short PR links in the changelog entries.
128+
* @param options.requirePrNumbers - Whether to require PR numbers for all commits. If true, commits without PR numbers are filtered out.
124129
*/
125130
async function update({
126131
changelogPath,
@@ -134,6 +139,7 @@ async function update({
134139
autoCategorize,
135140
useChangelogEntry,
136141
useShortPrLink,
142+
requirePrNumbers,
137143
}: UpdateOptions) {
138144
const changelogContent = await readChangelog(changelogPath);
139145

@@ -149,6 +155,7 @@ async function update({
149155
autoCategorize,
150156
useChangelogEntry,
151157
useShortPrLink,
158+
requirePrNumbers,
152159
});
153160

154161
if (newChangelogContent) {
@@ -357,6 +364,12 @@ async function main() {
357364
description: 'Use short PR links in the changelog entries',
358365
type: 'boolean',
359366
})
367+
.option('requirePrNumbers', {
368+
default: false,
369+
description:
370+
'Only include commits with PR numbers in the changelog. Commits without PR numbers will be filtered out',
371+
type: 'boolean',
372+
})
360373
.epilog(updateEpilog),
361374
)
362375
.command(
@@ -416,6 +429,7 @@ async function main() {
416429
prLinks,
417430
useChangelogEntry,
418431
useShortPrLink,
432+
requirePrNumbers,
419433
} = argv;
420434
let { currentVersion } = argv;
421435

@@ -547,6 +561,7 @@ async function main() {
547561
autoCategorize,
548562
useChangelogEntry: Boolean(useChangelogEntry),
549563
useShortPrLink: Boolean(useShortPrLink),
564+
requirePrNumbers: Boolean(requirePrNumbers),
550565
});
551566
} else if (command === 'validate') {
552567
let packageRename: PackageRename | undefined;

src/get-new-changes.ts

Lines changed: 59 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ export type AddNewCommitsOptions = {
1414
projectRootDirectory?: string;
1515
useChangelogEntry: boolean;
1616
useShortPrLink: boolean;
17+
requirePrNumbers?: boolean;
1718
};
1819

1920
// Get array of all ConventionalCommitType values
@@ -61,6 +62,22 @@ function removeConventionalCommitPrefixIfPresent(message: string) {
6162
return message.replace(regex, '');
6263
}
6364

65+
/**
66+
* Remove HTML comments from the given message.
67+
* Handles both complete comments (<!-- ... -->) and unclosed comments (<!-- ...).
68+
* This prevents PR template content from breaking the changelog markdown.
69+
*
70+
* @param message - The changelog entry message.
71+
* @returns The message without HTML comments.
72+
*/
73+
function stripHtmlComments(message: string): string {
74+
// Remove complete HTML comments (<!-- ... -->)
75+
let result = message.replace(/<!--[\s\S]*?-->/gu, '');
76+
// Remove unclosed HTML comments (<!-- without closing -->)
77+
result = result.replace(/<!--[\s\S]*$/gu, '');
78+
return result.trim();
79+
}
80+
6481
type Commit = {
6582
prNumber?: string;
6683
subject: string;
@@ -122,11 +139,12 @@ async function getCommits(
122139
const changelogMatch = body.match(/\nCHANGELOG entry:\s(\S.+?)\n\n/su);
123140

124141
if (changelogMatch) {
125-
const changelogEntry = changelogMatch[1].replace('\n', ' ');
142+
const changelogEntry = changelogMatch[1].replace('\n', ' ').trim();
126143

127144
description = changelogEntry; // This may be string 'null' to indicate no description
128145

129-
if (description !== 'null') {
146+
// Check for 'null' (case-insensitive) to exclude entries marked as no-changelog
147+
if (description.toLowerCase() !== 'null') {
130148
// Remove outer backticks if present. Example: `feat: new feature description` -> feat: new feature description
131149
description = removeOuterBackticksIfPresent(description);
132150

@@ -141,7 +159,8 @@ async function getCommits(
141159
description = subject.match(/^(.+)\s\(#\d+\)/u)?.[1] ?? '';
142160
}
143161

144-
if (description !== 'null') {
162+
// Filter out entries marked as no-changelog (case-insensitive null check)
163+
if (description.toLowerCase() !== 'null') {
145164
const prLabels = await getPrLabels(repoUrl, prNumber);
146165

147166
if (prLabels.includes('no-changelog')) {
@@ -233,6 +252,7 @@ function deduplicateCommits(
233252
* current git repository.
234253
* @param options.useChangelogEntry - Whether to use `CHANGELOG entry:` from the commit body and the no-changelog label.
235254
* @param options.useShortPrLink - Whether to use short PR links in the changelog entries.
255+
* @param options.requirePrNumbers - Whether to require PR numbers for all commits. If true, commits without PR numbers are filtered out.
236256
* @returns A list of new change entries to add to the changelog, based on commits made since the last release.
237257
*/
238258
export async function getNewChangeEntries({
@@ -243,6 +263,7 @@ export async function getNewChangeEntries({
243263
projectRootDirectory,
244264
useChangelogEntry,
245265
useShortPrLink,
266+
requirePrNumbers = false,
246267
}: AddNewCommitsOptions) {
247268
const commitRange =
248269
mostRecentTag === null ? 'HEAD' : `${mostRecentTag}..HEAD`;
@@ -256,8 +277,12 @@ export async function getNewChangeEntries({
256277
useChangelogEntry,
257278
);
258279

280+
const filteredPrCommits = requirePrNumbers
281+
? commits.filter((commit) => commit.prNumber !== undefined)
282+
: commits;
283+
259284
const newCommits = deduplicateCommits(
260-
commits,
285+
filteredPrCommits,
261286
loggedPrNumbers,
262287
loggedDescriptions,
263288
);
@@ -266,6 +291,11 @@ export async function getNewChangeEntries({
266291
// Handle edge case where PR description includes multiple CHANGELOG entries
267292
let newDescription = description?.replace(/CHANGELOG entry: /gu, '');
268293

294+
// Strip HTML comments that may come from PR templates to prevent broken markdown
295+
if (newDescription) {
296+
newDescription = stripHtmlComments(newDescription);
297+
}
298+
269299
// For merge commits, use the description for categorization because the subject
270300
// is "Merge pull request #123..." which would be incorrectly excluded
271301
const subjectForCategorization = isMergeCommit ? description : subject;
@@ -323,16 +353,31 @@ async function getPrLabels(
323353

324354
const { owner, repo } = getOwnerAndRepoFromUrl(repoUrl);
325355

326-
const { data: pullRequest } = await github.rest.pulls.get({
327-
owner,
328-
repo,
329-
// eslint-disable-next-line @typescript-eslint/naming-convention
330-
pull_number: Number(prNumber),
331-
});
332-
333-
if (pullRequest) {
334-
const labels = pullRequest.labels.map((label: any) => label.name);
335-
return labels;
356+
try {
357+
const { data: pullRequest } = await github.rest.pulls.get({
358+
owner,
359+
repo,
360+
// eslint-disable-next-line @typescript-eslint/naming-convention
361+
pull_number: Number(prNumber),
362+
});
363+
364+
if (pullRequest) {
365+
const labels = pullRequest.labels.map((label: any) => label.name);
366+
return labels;
367+
}
368+
} catch (error: unknown) {
369+
// If PR doesn't exist (404), return empty labels instead of throwing
370+
if (
371+
error instanceof Error &&
372+
'status' in error &&
373+
(error as { status: number }).status === 404
374+
) {
375+
console.warn(
376+
`PR #${prNumber} not found in ${owner}/${repo}, skipping label check`,
377+
);
378+
return [];
379+
}
380+
throw error;
336381
}
337382

338383
return [];

src/update-changelog.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,10 @@ export type UpdateChangelogOptions = {
112112
* Whether to use short PR links in the changelog entries.
113113
*/
114114
useShortPrLink?: boolean;
115+
/**
116+
* Whether to require PR numbers for all commits. If true, commits without PR numbers are filtered out.
117+
*/
118+
requirePrNumbers?: boolean;
115119
};
116120

117121
/**
@@ -140,6 +144,7 @@ export type UpdateChangelogOptions = {
140144
* based on commit message prefixes.
141145
* @param options.useChangelogEntry - Whether to use `CHANGELOG entry:` from the commit body and the no-changelog label.
142146
* @param options.useShortPrLink - Whether to use short PR links in the changelog.
147+
* @param options.requirePrNumbers - Whether to require PR numbers for all commits. If true, commits without PR numbers are filtered out.
143148
* @returns The updated changelog text.
144149
*/
145150
export async function updateChangelog({
@@ -154,6 +159,7 @@ export async function updateChangelog({
154159
autoCategorize,
155160
useChangelogEntry = false,
156161
useShortPrLink = false,
162+
requirePrNumbers = false,
157163
}: UpdateChangelogOptions): Promise<string | undefined> {
158164
const changelog = parseChangelog({
159165
changelogContent,
@@ -204,6 +210,7 @@ export async function updateChangelog({
204210
projectRootDirectory,
205211
useChangelogEntry,
206212
useShortPrLink,
213+
requirePrNumbers,
207214
});
208215

209216
for (const entry of newChangeEntries.reverse()) {

0 commit comments

Comments
 (0)