From 595e18080ae9ea37e92c874537144f69a817f6eb Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Thu, 15 Jan 2026 14:24:50 +0000 Subject: [PATCH] fix(changelog): Warn when custom release.yml lacks semver fields When a custom .github/release.yml is used without semver fields on categories, PRs appear in the changelog but don't contribute to version bump detection. This causes the changelog preview to show "None" for semver impact. Changes: - Add warning when normalizing custom configs with missing semver fields - Update docs to clarify semver field requirement for version detection - Add tests for the warning behavior Fixes getsentry/sentry-wizard#1191 investigation --- docs/src/content/docs/configuration.md | 8 + docs/src/content/docs/github-actions.md | 72 +- .../__tests__/changelog-generate.test.ts | 622 +++++++++++------- .../changelog-semver-warning.test.ts | 195 ++++++ src/utils/__tests__/changelog-utils.test.ts | 216 ++++-- src/utils/changelog.ts | 184 ++++-- 6 files changed, 884 insertions(+), 413 deletions(-) create mode 100644 src/utils/__tests__/changelog-semver-warning.test.ts diff --git a/docs/src/content/docs/configuration.md b/docs/src/content/docs/configuration.md index 54f53a24..949db431 100644 --- a/docs/src/content/docs/configuration.md +++ b/docs/src/content/docs/configuration.md @@ -31,6 +31,7 @@ The command is executed with the following environment variables: The script should: - Use these environment variables to perform version replacement +- Replace version occurrences - Not commit changes - Not change git state @@ -105,6 +106,13 @@ Craft extends GitHub's format with two additional fields: | `commit_patterns` | Array of regex patterns to match commit/PR titles (in addition to labels) | | `semver` | Version bump type for auto-versioning: `major`, `minor`, or `patch` | +:::caution[Required for Version Detection] +The `semver` field is required for Craft's automatic version detection to work. +If you define a custom `.github/release.yml` without `semver` fields, PRs will +still appear in the changelog but won't contribute to suggested version bumps. +The [changelog preview](/github-actions/#changelog-preview) will show "None" for semver impact. +::: + #### Default Configuration If `.github/release.yml` doesn't exist, Craft uses these defaults based on [Conventional Commits](https://www.conventionalcommits.org/): diff --git a/docs/src/content/docs/github-actions.md b/docs/src/content/docs/github-actions.md index 425d2e6a..6abee55d 100644 --- a/docs/src/content/docs/github-actions.md +++ b/docs/src/content/docs/github-actions.md @@ -11,10 +11,10 @@ For a real-world example of using Craft's GitHub Actions, see the [getsentry/pub Craft offers two ways to automate releases in GitHub Actions: -| Option | Best For | Flexibility | -|--------|----------|-------------| -| **Reusable Workflow** | Quick setup, standard release flow | Low - runs as a complete job | -| **Composite Action** | Custom workflows, pre/post steps | High - composable with other steps | +| Option | Best For | Flexibility | +| --------------------- | ---------------------------------- | ---------------------------------- | +| **Reusable Workflow** | Quick setup, standard release flow | Low - runs as a complete job | +| **Composite Action** | Custom workflows, pre/post steps | High - composable with other steps | ### Option 1: Reusable Workflow (Recommended) @@ -39,27 +39,27 @@ jobs: #### Workflow Inputs -| Input | Description | Default | -|-------|-------------|---------| -| `version` | Version to release. Can be a semver string (e.g., "1.2.3"), a bump type ("major", "minor", "patch"), or "auto" for automatic detection. | Uses `versioning.policy` from config | -| `merge_target` | Target branch to merge into. | Default branch | -| `force` | Force a release even when there are release-blockers. | `false` | -| `blocker_label` | Label that blocks releases. | `release-blocker` | -| `publish_repo` | Repository for publish issues (owner/repo format). | `{owner}/publish` | -| `git_user_name` | Git committer name. | GitHub actor | -| `git_user_email` | Git committer email. | Actor's noreply email | -| `path` | The path that Craft will run inside. | `.` | -| `craft_config_from_merge_target` | Use the craft config from the merge target branch. | `false` | +| Input | Description | Default | +| -------------------------------- | --------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------ | +| `version` | Version to release. Can be a semver string (e.g., "1.2.3"), a bump type ("major", "minor", "patch"), or "auto" for automatic detection. | Uses `versioning.policy` from config | +| `merge_target` | Target branch to merge into. | Default branch | +| `force` | Force a release even when there are release-blockers. | `false` | +| `blocker_label` | Label that blocks releases. | `release-blocker` | +| `publish_repo` | Repository for publish issues (owner/repo format). | `{owner}/publish` | +| `git_user_name` | Git committer name. | GitHub actor | +| `git_user_email` | Git committer email. | Actor's noreply email | +| `path` | The path that Craft will run inside. | `.` | +| `craft_config_from_merge_target` | Use the craft config from the merge target branch. | `false` | #### Workflow Outputs -| Output | Description | -|--------|-------------| -| `version` | The resolved version being released | -| `branch` | The release branch name | -| `sha` | The commit SHA on the release branch | +| Output | Description | +| -------------- | -------------------------------------------- | +| `version` | The resolved version being released | +| `branch` | The release branch name | +| `sha` | The commit SHA on the release branch | | `previous_tag` | The tag before this release (for diff links) | -| `changelog` | The changelog for this release | +| `changelog` | The changelog for this release | ### Option 2: Composite Action @@ -106,7 +106,7 @@ When using auto-versioning, Craft analyzes conventional commits to determine the name: Auto Release on: schedule: - - cron: '0 10 * * 1' # Every Monday at 10 AM + - cron: '0 10 * * 1' # Every Monday at 10 AM jobs: release: @@ -142,8 +142,8 @@ jobs: ### Inputs -| Input | Description | Default | -|-------|-------------|---------| +| Input | Description | Default | +| --------------- | ----------------------------------------- | -------- | | `craft-version` | Version of Craft to use (tag or "latest") | `latest` | ### Pinning a Specific Version @@ -157,7 +157,7 @@ jobs: changelog-preview: uses: getsentry/craft/.github/workflows/changelog-preview.yml@v2 with: - craft-version: "2.15.0" + craft-version: '2.15.0' secrets: inherit ``` @@ -171,6 +171,12 @@ jobs: 6. **Posts a comment** - Creates or updates a comment on the PR with the changelog preview 7. **Auto-updates** - The comment is automatically updated when you update the PR (push commits, edit title/description, or change labels) +:::note +The version bump suggestion requires categories in your `.github/release.yml` to have +`semver` fields defined. Without them, the suggested bump will show as "None". +See [Auto Mode configuration](/configuration/#auto-mode) for details. +::: + ### Example Comment The workflow posts a comment like this: @@ -205,6 +211,7 @@ Entries from this PR are highlighted with a left border (blockquote style). ### PR Trigger Types The workflow supports these PR event types: + - `opened` - When a PR is created - `synchronize` - When new commits are pushed - `reopened` - When a closed PR is reopened @@ -216,13 +223,16 @@ The workflow supports these PR event types: The workflow requires specific permissions and secrets to function correctly: **Permissions** (required): + - `contents: read` - Allows the workflow to checkout your repository and read git history for changelog generation - `pull-requests: write` - Allows the workflow to post and update comments on pull requests **Secrets**: + - `secrets: inherit` - Passes your repository's `GITHUB_TOKEN` to the workflow, ensuring it has access to your repository (especially important for private repositories) **Repository Setup**: + - The repository should have a git history with tags for the changelog to be meaningful :::note[Why are these permissions needed?] @@ -248,13 +258,13 @@ You can configure labels to exclude PRs from the changelog. In your `.craft.yml` ```yaml changelog: categories: - - title: "New Features ✨" - labels: ["feature", "enhancement"] - - title: "Bug Fixes 🐛" - labels: ["bug", "fix"] + - title: 'New Features ✨' + labels: ['feature', 'enhancement'] + - title: 'Bug Fixes 🐛' + labels: ['bug', 'fix'] exclude: - labels: ["skip-changelog", "dependencies"] - authors: ["dependabot[bot]", "renovate[bot]"] + labels: ['skip-changelog', 'dependencies'] + authors: ['dependabot[bot]', 'renovate[bot]'] ``` PRs with the `skip-changelog` label or from excluded authors will not appear in the changelog. diff --git a/src/utils/__tests__/changelog-generate.test.ts b/src/utils/__tests__/changelog-generate.test.ts index 741723cd..1007ac2c 100644 --- a/src/utils/__tests__/changelog-generate.test.ts +++ b/src/utils/__tests__/changelog-generate.test.ts @@ -37,6 +37,7 @@ import { generateChangesetFromGit, generateChangelogWithHighlight, clearChangesetCache, + clearReleaseConfigCache, } from '../changelog'; import { type TestCommit } from './fixtures/changelog'; @@ -59,6 +60,7 @@ describe('generateChangesetFromGit', () => { beforeEach(() => { vi.resetAllMocks(); clearChangesetCache(); + clearReleaseConfigCache(); mockClient = vi.fn(); (getGitHubClient as MockedFunction).mockReturnValue( { @@ -824,20 +826,29 @@ changelog: it('cancels out a simple revert via SHA match', async () => { // Commit A and Revert A should cancel each other out // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456', - title: 'Revert "feat: add new feature"', - body: 'This reverts commit abc123.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'abc123', - title: 'feat: add new feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'def456', + title: 'Revert "feat: add new feature"', + body: 'This reverts commit abc123.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'abc123', + title: 'feat: add new feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // Both commits should cancel out, resulting in empty changelog expect(result.changelog).toBe(''); @@ -847,20 +858,29 @@ changelog: it('cancels out a simple revert via title fallback', async () => { // When no SHA in body, fall back to title matching // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456', - title: 'Revert "feat: add new feature"', - body: '', // No SHA in body - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'abc123', - title: 'feat: add new feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'def456', + title: 'Revert "feat: add new feature"', + body: '', // No SHA in body + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'abc123', + title: 'feat: add new feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); expect(result.changelog).toBe(''); expect(result.bumpType).toBeNull(); @@ -868,18 +888,26 @@ changelog: it('keeps standalone revert when original is not in changelog', async () => { // Revert without corresponding original commit stays as bug fix - setup([ - { - hash: 'def456', - title: 'Revert "feat: add feature from previous release"', - body: 'This reverts commit oldsha123.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - ], null); + setup( + [ + { + hash: 'def456', + title: 'Revert "feat: add feature from previous release"', + body: 'This reverts commit oldsha123.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // Revert should appear in Bug Fixes category with full title expect(result.changelog).toContain('Bug Fixes'); - expect(result.changelog).toContain('Revert "feat: add feature from previous release"'); + expect(result.changelog).toContain( + 'Revert "feat: add feature from previous release"', + ); expect(result.bumpType).toBe('patch'); }); @@ -887,26 +915,38 @@ changelog: // A -> B (Revert A) -> C (Revert B) // Expected: C cancels B, A remains // Commits in newest-first order (git log order) - setup([ - { - hash: 'ccc333', - title: 'Revert "Revert "feat: add feature""', - body: 'This reverts commit bbb222.', - pr: { local: '3', remote: { number: '3', author: { login: 'charlie' } } }, - }, - { - hash: 'bbb222', - title: 'Revert "feat: add feature"', - body: 'This reverts commit aaa111.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'aaa111', - title: 'feat: add feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'ccc333', + title: 'Revert "Revert "feat: add feature""', + body: 'This reverts commit bbb222.', + pr: { + local: '3', + remote: { number: '3', author: { login: 'charlie' } }, + }, + }, + { + hash: 'bbb222', + title: 'Revert "feat: add feature"', + body: 'This reverts commit aaa111.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'aaa111', + title: 'feat: add feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // B and C cancel out, A remains expect(result.changelog).toContain('New Features'); @@ -920,32 +960,47 @@ changelog: // Processing newest first: // D cancels C, B cancels A -> nothing remains // Commits in newest-first order (git log order) - setup([ - { - hash: 'ddd444', - title: 'Revert "Revert "Revert "feat: add feature"""', - body: 'This reverts commit ccc333.', - pr: { local: '4', remote: { number: '4', author: { login: 'dave' } } }, - }, - { - hash: 'ccc333', - title: 'Revert "Revert "feat: add feature""', - body: 'This reverts commit bbb222.', - pr: { local: '3', remote: { number: '3', author: { login: 'charlie' } } }, - }, - { - hash: 'bbb222', - title: 'Revert "feat: add feature"', - body: 'This reverts commit aaa111.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'aaa111', - title: 'feat: add feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'ddd444', + title: 'Revert "Revert "Revert "feat: add feature"""', + body: 'This reverts commit ccc333.', + pr: { + local: '4', + remote: { number: '4', author: { login: 'dave' } }, + }, + }, + { + hash: 'ccc333', + title: 'Revert "Revert "feat: add feature""', + body: 'This reverts commit bbb222.', + pr: { + local: '3', + remote: { number: '3', author: { login: 'charlie' } }, + }, + }, + { + hash: 'bbb222', + title: 'Revert "feat: add feature"', + body: 'This reverts commit aaa111.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'aaa111', + title: 'feat: add feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // D cancels C, B cancels A -> empty expect(result.changelog).toBe(''); @@ -957,38 +1012,56 @@ changelog: // Processing newest first: // E cancels D, C cancels B, A remains // Commits in newest-first order (git log order) - setup([ - { - hash: 'eee555', - title: 'Revert "Revert "Revert "Revert "feat: add feature""""', - body: 'This reverts commit ddd444.', - pr: { local: '5', remote: { number: '5', author: { login: 'eve' } } }, - }, - { - hash: 'ddd444', - title: 'Revert "Revert "Revert "feat: add feature"""', - body: 'This reverts commit ccc333.', - pr: { local: '4', remote: { number: '4', author: { login: 'dave' } } }, - }, - { - hash: 'ccc333', - title: 'Revert "Revert "feat: add feature""', - body: 'This reverts commit bbb222.', - pr: { local: '3', remote: { number: '3', author: { login: 'charlie' } } }, - }, - { - hash: 'bbb222', - title: 'Revert "feat: add feature"', - body: 'This reverts commit aaa111.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'aaa111', - title: 'feat: add feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'eee555', + title: 'Revert "Revert "Revert "Revert "feat: add feature""""', + body: 'This reverts commit ddd444.', + pr: { + local: '5', + remote: { number: '5', author: { login: 'eve' } }, + }, + }, + { + hash: 'ddd444', + title: 'Revert "Revert "Revert "feat: add feature"""', + body: 'This reverts commit ccc333.', + pr: { + local: '4', + remote: { number: '4', author: { login: 'dave' } }, + }, + }, + { + hash: 'ccc333', + title: 'Revert "Revert "feat: add feature""', + body: 'This reverts commit bbb222.', + pr: { + local: '3', + remote: { number: '3', author: { login: 'charlie' } }, + }, + }, + { + hash: 'bbb222', + title: 'Revert "feat: add feature"', + body: 'This reverts commit aaa111.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'aaa111', + title: 'feat: add feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // E cancels D, C cancels B, A remains expect(result.changelog).toContain('New Features'); @@ -1001,26 +1074,38 @@ changelog: // Two commits with same title, revert SHA points to first one // Only first one should be canceled, second remains // Commits in newest-first order (git log order) - setup([ - { - hash: 'ccc333', - title: 'Revert "feat: add feature"', - body: 'This reverts commit aaa111.', // SHA points to first - pr: { local: '3', remote: { number: '3', author: { login: 'charlie' } } }, - }, - { - hash: 'bbb222', - title: 'feat: add feature', // Same title as first - body: '', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'aaa111', - title: 'feat: add feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'ccc333', + title: 'Revert "feat: add feature"', + body: 'This reverts commit aaa111.', // SHA points to first + pr: { + local: '3', + remote: { number: '3', author: { login: 'charlie' } }, + }, + }, + { + hash: 'bbb222', + title: 'feat: add feature', // Same title as first + body: '', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'aaa111', + title: 'feat: add feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // First feat and revert cancel, second feat remains expect(result.changelog).toContain('Add feature'); @@ -1031,20 +1116,29 @@ changelog: it('handles revert with PR number suffix in title', async () => { // GitHub often includes PR number in title like: Revert "feat: add feature (#1)" // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456', - title: 'Revert "feat: add feature (#1)" (#2)', - body: 'This reverts commit abc123.', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'abc123', - title: 'feat: add feature (#1)', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + setup( + [ + { + hash: 'def456', + title: 'Revert "feat: add feature (#1)" (#2)', + body: 'This reverts commit abc123.', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'abc123', + title: 'feat: add feature (#1)', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); expect(result.changelog).toBe(''); }); @@ -1052,27 +1146,33 @@ changelog: it('uses PR title for matching when available', async () => { // PR title differs from commit title, should use PR title for matching // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456', - title: 'Revert "feat: add feature"', // Matches PR title, not commit title - body: '', - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'abc123', - title: 'wip commit message', - body: '', - pr: { - local: '1', - remote: { - number: '1', - author: { login: 'alice' }, - title: 'feat: add feature', // PR title is different + setup( + [ + { + hash: 'def456', + title: 'Revert "feat: add feature"', // Matches PR title, not commit title + body: '', + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, }, }, - }, - ], null); + { + hash: 'abc123', + title: 'wip commit message', + body: '', + pr: { + local: '1', + remote: { + number: '1', + author: { login: 'alice' }, + title: 'feat: add feature', // PR title is different + }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); expect(result.changelog).toBe(''); }); @@ -1080,25 +1180,34 @@ changelog: it('extracts SHA from body with additional explanation text', async () => { // Revert body often contains explanation in addition to the "This reverts commit" line // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456abc123', - title: 'Revert "feat: add new feature"', - body: `This reverts commit abc123def456. + setup( + [ + { + hash: 'def456abc123', + title: 'Revert "feat: add new feature"', + body: `This reverts commit abc123def456. The feature caused performance issues in production. We need to investigate further before re-enabling. See incident report: https://example.com/incident/123`, - pr: { local: '2', remote: { number: '2', author: { login: 'bob' } } }, - }, - { - hash: 'abc123def456', - title: 'feat: add new feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + pr: { + local: '2', + remote: { number: '2', author: { login: 'bob' } }, + }, + }, + { + hash: 'abc123def456', + title: 'feat: add new feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // Both should cancel out despite the additional text in the body expect(result.changelog).toBe(''); @@ -1109,27 +1218,33 @@ See incident report: https://example.com/incident/123`, // Sometimes the title may not follow the "Revert "..."" format, // but the body still contains "This reverts commit " // Commits in newest-first order (git log order) - setup([ - { - hash: 'def456abc123', - title: 'fix: undo the new feature due to issues', // Non-standard revert title - body: '', - pr: { - local: '2', - remote: { - number: '2', - author: { login: 'bob' }, - body: 'This reverts commit abc123def456.\n\nThe feature caused problems.', + setup( + [ + { + hash: 'def456abc123', + title: 'fix: undo the new feature due to issues', // Non-standard revert title + body: '', + pr: { + local: '2', + remote: { + number: '2', + author: { login: 'bob' }, + body: 'This reverts commit abc123def456.\n\nThe feature caused problems.', + }, }, }, - }, - { - hash: 'abc123def456', - title: 'feat: add new feature', - body: '', - pr: { local: '1', remote: { number: '1', author: { login: 'alice' } } }, - }, - ], null); + { + hash: 'abc123def456', + title: 'feat: add new feature', + body: '', + pr: { + local: '1', + remote: { number: '1', author: { login: 'alice' } }, + }, + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); // Both should cancel out because body contains the revert magic string with SHA expect(result.changelog).toBe(''); @@ -1142,47 +1257,50 @@ See incident report: https://example.com/incident/123`, // When using rebase-merge, all individual commits from a PR are added to // the base branch and each gets associated with the same PR number. // We should only show the PR once in the changelog. - setup([ - { - hash: 'commit1', - title: 'feat(ui): add button component', - body: '', - pr: { - local: '42', - remote: { - number: '42', - title: 'feat(ui): add button component', - author: { login: 'alice' }, + setup( + [ + { + hash: 'commit1', + title: 'feat(ui): add button component', + body: '', + pr: { + local: '42', + remote: { + number: '42', + title: 'feat(ui): add button component', + author: { login: 'alice' }, + }, }, }, - }, - { - hash: 'commit2', - title: 'feat(ui): add button styles', - body: '', - pr: { - local: '42', - remote: { - number: '42', - title: 'feat(ui): add button component', // Same PR, same title from API - author: { login: 'alice' }, + { + hash: 'commit2', + title: 'feat(ui): add button styles', + body: '', + pr: { + local: '42', + remote: { + number: '42', + title: 'feat(ui): add button component', // Same PR, same title from API + author: { login: 'alice' }, + }, }, }, - }, - { - hash: 'commit3', - title: 'feat(ui): add button tests', - body: '', - pr: { - local: '42', - remote: { - number: '42', - title: 'feat(ui): add button component', // Same PR, same title from API - author: { login: 'alice' }, + { + hash: 'commit3', + title: 'feat(ui): add button tests', + body: '', + pr: { + local: '42', + remote: { + number: '42', + title: 'feat(ui): add button component', // Same PR, same title from API + author: { login: 'alice' }, + }, }, }, - }, - ], null); + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); @@ -1194,20 +1312,23 @@ See incident report: https://example.com/incident/123`, it('keeps commits without PR association even if they share hashes', async () => { // Commits without PR association should all be kept - setup([ - { - hash: 'commit1', - title: 'chore: update dependencies', - body: '', - // No PR association - }, - { - hash: 'commit2', - title: 'chore: fix typo', - body: '', - // No PR association - }, - ], null); + setup( + [ + { + hash: 'commit1', + title: 'chore: update dependencies', + body: '', + // No PR association + }, + { + hash: 'commit2', + title: 'chore: fix typo', + body: '', + // No PR association + }, + ], + null, + ); const result = await generateChangesetFromGit(dummyGit, '1.0.0', 10); @@ -1234,6 +1355,7 @@ describe('generateChangelogWithHighlight', () => { beforeEach(() => { vi.resetAllMocks(); clearChangesetCache(); + clearReleaseConfigCache(); mockGraphqlClient = vi.fn(); mockRestClient = { pulls: { get: vi.fn() }, diff --git a/src/utils/__tests__/changelog-semver-warning.test.ts b/src/utils/__tests__/changelog-semver-warning.test.ts new file mode 100644 index 00000000..09ea59c6 --- /dev/null +++ b/src/utils/__tests__/changelog-semver-warning.test.ts @@ -0,0 +1,195 @@ +import { vi, beforeEach, describe, it, expect } from 'vitest'; + +// Mock the logger before importing changelog +vi.mock('../../logger'); + +// Mock the config module to control getConfigFileDir +vi.mock('../../config', () => ({ + getConfigFileDir: vi.fn(), + getGlobalGitHubConfig: vi.fn(), + getChangelogConfig: vi.fn(), +})); + +// Mock fs to control file reading +vi.mock('fs', async () => { + const actual = await vi.importActual('fs'); + return { + ...actual, + readFileSync: vi.fn(), + }; +}); + +describe('getNormalizedReleaseConfig semver warnings', () => { + beforeEach(async () => { + vi.resetModules(); + vi.clearAllMocks(); + }); + + it('warns when custom config has categories without semver fields', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + // Setup: custom config file exists with missing semver fields + vi.mocked(getConfigFileDir).mockReturnValue('/test/repo'); + vi.mocked(readFileSync).mockReturnValue(` +changelog: + categories: + - title: Features + labels: + - enhancement + - title: Bug Fixes + labels: + - bug +`); + + // Import fresh module after mocks are set up + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + getNormalizedReleaseConfig(); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('Features'), + ); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('Bug Fixes'), + ); + expect(logger.warn).toHaveBeenCalledWith(expect.stringContaining('semver')); + }); + + it('does not warn when using default config (no file)', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + // Setup: no config file dir + vi.mocked(getConfigFileDir).mockReturnValue(null); + + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + getNormalizedReleaseConfig(); + + expect(logger.warn).not.toHaveBeenCalled(); + expect(readFileSync).not.toHaveBeenCalled(); + }); + + it('does not warn when all categories have semver fields', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + vi.mocked(getConfigFileDir).mockReturnValue('/test/repo'); + vi.mocked(readFileSync).mockReturnValue(` +changelog: + categories: + - title: Features + labels: + - enhancement + semver: minor + - title: Bug Fixes + labels: + - bug + semver: patch +`); + + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + getNormalizedReleaseConfig(); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('only warns about categories missing semver', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + vi.mocked(getConfigFileDir).mockReturnValue('/test/repo'); + vi.mocked(readFileSync).mockReturnValue(` +changelog: + categories: + - title: Features + labels: + - enhancement + semver: minor + - title: Internal + labels: + - internal +`); + + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + getNormalizedReleaseConfig(); + + expect(logger.warn).toHaveBeenCalledTimes(1); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining('Internal'), + ); + expect(logger.warn).toHaveBeenCalledWith( + expect.not.stringContaining('Features'), + ); + }); + + it('only warns once due to memoization', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + vi.mocked(getConfigFileDir).mockReturnValue('/test/repo'); + vi.mocked(readFileSync).mockReturnValue(` +changelog: + categories: + - title: Features + labels: + - enhancement +`); + + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + // Call multiple times + getNormalizedReleaseConfig(); + getNormalizedReleaseConfig(); + getNormalizedReleaseConfig(); + + // Should only warn once due to memoization + expect(logger.warn).toHaveBeenCalledTimes(1); + }); + + it('includes documentation URL in warning message', async () => { + const { readFileSync } = await import('fs'); + const { getConfigFileDir } = await import('../../config'); + const { logger } = await import('../../logger'); + + vi.mocked(getConfigFileDir).mockReturnValue('/test/repo'); + vi.mocked(readFileSync).mockReturnValue(` +changelog: + categories: + - title: Features + labels: + - enhancement +`); + + const { getNormalizedReleaseConfig, clearReleaseConfigCache } = + await import('../changelog'); + clearReleaseConfigCache(); + + getNormalizedReleaseConfig(); + + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + 'https://getsentry.github.io/craft/configuration/#auto-mode', + ), + ); + }); +}); diff --git a/src/utils/__tests__/changelog-utils.test.ts b/src/utils/__tests__/changelog-utils.test.ts index 3fd20129..b676c842 100644 --- a/src/utils/__tests__/changelog-utils.test.ts +++ b/src/utils/__tests__/changelog-utils.test.ts @@ -1,9 +1,16 @@ -import { vi, type Mock, type MockInstance, type Mocked, type MockedFunction } from 'vitest'; +import { + vi, + type Mock, + type MockInstance, + type Mocked, + type MockedFunction, +} from 'vitest'; /** * Tests for changelog utility functions. * - shouldExcludePR: Checks if a PR should be excluded from changelog * - shouldSkipCurrentPR: Checks if current PR should skip changelog generation * - getBumpTypeForPR: Determines the version bump type for a PR + * - normalizeReleaseConfig: Normalizes release config and warns about missing semver fields * * Note: shouldSkipCurrentPR and getBumpTypeForPR read config internally, * so they only take the PRInfo argument. More comprehensive tests are @@ -19,11 +26,16 @@ import { extractRevertedTitle, extractRevertedSha, processReverts, + getNormalizedReleaseConfig, + clearReleaseConfigCache, SKIP_CHANGELOG_MAGIC_WORD, BODY_IN_CHANGELOG_MAGIC_WORD, type CurrentPRInfo, } from '../changelog'; +// Mock the logger to capture warnings +vi.mock('../../logger'); + describe('shouldExcludePR', () => { // Config must match NormalizedReleaseConfig structure const baseConfig = { @@ -36,48 +48,41 @@ describe('shouldExcludePR', () => { }; it('returns true when PR has excluded label', () => { - expect(shouldExcludePR( - new Set(['bug', 'skip-changelog']), - 'alice', - baseConfig as any, - '' - )).toBe(true); + expect( + shouldExcludePR( + new Set(['bug', 'skip-changelog']), + 'alice', + baseConfig as any, + '', + ), + ).toBe(true); }); it('returns true when PR has excluded author', () => { - expect(shouldExcludePR( - new Set(['bug']), - 'dependabot', - baseConfig as any, - '' - )).toBe(true); + expect( + shouldExcludePR(new Set(['bug']), 'dependabot', baseConfig as any, ''), + ).toBe(true); }); it('returns false when PR has no exclusion criteria', () => { - expect(shouldExcludePR( - new Set(['bug']), - 'alice', - baseConfig as any, - '' - )).toBe(false); + expect( + shouldExcludePR(new Set(['bug']), 'alice', baseConfig as any, ''), + ).toBe(false); }); it('returns true when body contains skip magic word', () => { - expect(shouldExcludePR( - new Set(['bug']), - 'alice', - baseConfig as any, - `Some text\n${SKIP_CHANGELOG_MAGIC_WORD}\nMore text` - )).toBe(true); + expect( + shouldExcludePR( + new Set(['bug']), + 'alice', + baseConfig as any, + `Some text\n${SKIP_CHANGELOG_MAGIC_WORD}\nMore text`, + ), + ).toBe(true); }); it('returns false when config is null', () => { - expect(shouldExcludePR( - new Set(['bug']), - 'alice', - null, - '' - )).toBe(false); + expect(shouldExcludePR(new Set(['bug']), 'alice', null, '')).toBe(false); }); }); @@ -99,7 +104,10 @@ describe('shouldSkipCurrentPR', () => { }); it('returns true when PR body contains skip magic word', () => { - const prInfo = { ...basePRInfo, body: `Some text\n${SKIP_CHANGELOG_MAGIC_WORD}` }; + const prInfo = { + ...basePRInfo, + body: `Some text\n${SKIP_CHANGELOG_MAGIC_WORD}`, + }; expect(shouldSkipCurrentPR(prInfo)).toBe(true); }); }); @@ -156,31 +164,31 @@ describe('stripTitle', () => { it('strips the type prefix', () => { expect(stripTitle('feat: add endpoint', pattern, false)).toBe( - 'Add endpoint' + 'Add endpoint', ); }); it('strips type and scope when preserveScope is false', () => { expect(stripTitle('feat(api): add endpoint', pattern, false)).toBe( - 'Add endpoint' + 'Add endpoint', ); }); it('preserves scope when preserveScope is true', () => { expect(stripTitle('feat(api): add endpoint', pattern, true)).toBe( - '(api) Add endpoint' + '(api) Add endpoint', ); }); it('capitalizes first letter after stripping', () => { expect(stripTitle('feat: lowercase start', pattern, false)).toBe( - 'Lowercase start' + 'Lowercase start', ); }); it('handles already capitalized content', () => { expect(stripTitle('feat: Already Capitalized', pattern, false)).toBe( - 'Already Capitalized' + 'Already Capitalized', ); }); @@ -191,17 +199,17 @@ describe('stripTitle', () => { it('handles breaking change indicator', () => { const breakingPattern = /^(?feat(?:\((?[^)]+)\))?!:\s*)/; expect(stripTitle('feat!: breaking change', breakingPattern, false)).toBe( - 'Breaking change' + 'Breaking change', ); expect( - stripTitle('feat(api)!: breaking api change', breakingPattern, false) + stripTitle('feat(api)!: breaking api change', breakingPattern, false), ).toBe('Breaking api change'); }); it('does not strip when pattern has no type group', () => { const noGroupPattern = /^feat(?:\([^)]+\))?!?:\s*/; expect(stripTitle('feat: add endpoint', noGroupPattern, false)).toBe( - 'feat: add endpoint' + 'feat: add endpoint', ); }); }); @@ -211,7 +219,7 @@ describe('stripTitle', () => { it('returns original if pattern is undefined', () => { expect(stripTitle('feat: add endpoint', undefined, false)).toBe( - 'feat: add endpoint' + 'feat: add endpoint', ); }); @@ -222,17 +230,17 @@ describe('stripTitle', () => { it('handles scope with special characters', () => { expect(stripTitle('feat(my-api): add endpoint', pattern, true)).toBe( - '(my-api) Add endpoint' + '(my-api) Add endpoint', ); expect(stripTitle('feat(my_api): add endpoint', pattern, true)).toBe( - '(my_api) Add endpoint' + '(my_api) Add endpoint', ); }); it('does not preserve scope when scope is not captured', () => { const noScopePattern = /^(?feat(?:\([^)]+\))?!?:\s*)/; expect(stripTitle('feat(api): add endpoint', noScopePattern, true)).toBe( - 'Add endpoint' + 'Add endpoint', ); }); }); @@ -241,20 +249,20 @@ describe('stripTitle', () => { it('works with fix type', () => { const pattern = /^(?fix(?:\((?[^)]+)\))?!?:\s*)/; expect(stripTitle('fix(core): resolve bug', pattern, false)).toBe( - 'Resolve bug' + 'Resolve bug', ); expect(stripTitle('fix(core): resolve bug', pattern, true)).toBe( - '(core) Resolve bug' + '(core) Resolve bug', ); }); it('works with docs type', () => { const pattern = /^(?docs?(?:\((?[^)]+)\))?!?:\s*)/; expect(stripTitle('docs(readme): update docs', pattern, false)).toBe( - 'Update docs' + 'Update docs', ); expect(stripTitle('doc(readme): update docs', pattern, false)).toBe( - 'Update docs' + 'Update docs', ); }); @@ -262,13 +270,13 @@ describe('stripTitle', () => { const pattern = /^(?(?:build|refactor|meta|chore|ci|ref|perf)(?:\((?[^)]+)\))?!?:\s*)/; expect(stripTitle('chore(deps): update deps', pattern, false)).toBe( - 'Update deps' + 'Update deps', ); expect(stripTitle('build(ci): fix pipeline', pattern, false)).toBe( - 'Fix pipeline' + 'Fix pipeline', ); expect(stripTitle('refactor(api): simplify logic', pattern, true)).toBe( - '(api) Simplify logic' + '(api) Simplify logic', ); }); }); @@ -285,7 +293,9 @@ describe('isRevertCommit', () => { }); it('returns true when body contains revert magic string', () => { - expect(isRevertCommit('fix: undo feature', 'This reverts commit abc123def.')).toBe(true); + expect( + isRevertCommit('fix: undo feature', 'This reverts commit abc123def.'), + ).toBe(true); }); it('returns false when neither title nor body indicates revert', () => { @@ -295,18 +305,26 @@ describe('isRevertCommit', () => { it('handles case-insensitive text matching in body', () => { // The "This reverts commit" text is matched case-insensitively // (SHAs in git are always lowercase, so we only test the text part) - expect(isRevertCommit('fix: undo', 'THIS REVERTS COMMIT abc123def.')).toBe(true); - expect(isRevertCommit('fix: undo', 'this Reverts Commit abc123def.')).toBe(true); + expect(isRevertCommit('fix: undo', 'THIS REVERTS COMMIT abc123def.')).toBe( + true, + ); + expect(isRevertCommit('fix: undo', 'this Reverts Commit abc123def.')).toBe( + true, + ); }); }); describe('extractRevertedSha', () => { it('extracts SHA from standard git revert message', () => { - expect(extractRevertedSha('This reverts commit abc123def456.')).toBe('abc123def456'); + expect(extractRevertedSha('This reverts commit abc123def456.')).toBe( + 'abc123def456', + ); }); it('extracts SHA without trailing period', () => { - expect(extractRevertedSha('This reverts commit abc123def456')).toBe('abc123def456'); + expect(extractRevertedSha('This reverts commit abc123def456')).toBe( + 'abc123def456', + ); }); it('extracts SHA from body with additional text', () => { @@ -338,46 +356,52 @@ describe('extractRevertedTitle', () => { // after 'Revert ' and the last " before the end/PR suffix. it('extracts title from simple revert', () => { - expect(extractRevertedTitle('Revert "feat: add feature"')).toBe('feat: add feature'); + expect(extractRevertedTitle('Revert "feat: add feature"')).toBe( + 'feat: add feature', + ); }); it('extracts title from revert with PR suffix', () => { - expect(extractRevertedTitle('Revert "feat: add feature" (#123)')).toBe('feat: add feature'); + expect(extractRevertedTitle('Revert "feat: add feature" (#123)')).toBe( + 'feat: add feature', + ); }); it('extracts title from double revert (nested quotes)', () => { // Revert "Revert "feat: add feature"" // The greedy .+ matches: Revert "feat: add feature" expect(extractRevertedTitle('Revert "Revert "feat: add feature""')).toBe( - 'Revert "feat: add feature"' + 'Revert "feat: add feature"', ); }); it('extracts title from triple revert (deeply nested quotes)', () => { // Revert "Revert "Revert "feat: add feature""" - expect(extractRevertedTitle('Revert "Revert "Revert "feat: add feature"""')).toBe( - 'Revert "Revert "feat: add feature""' - ); + expect( + extractRevertedTitle('Revert "Revert "Revert "feat: add feature"""'), + ).toBe('Revert "Revert "feat: add feature""'); }); it('extracts title from quadruple revert', () => { // Revert "Revert "Revert "Revert "feat: add feature"""" - expect(extractRevertedTitle('Revert "Revert "Revert "Revert "feat: add feature""""')).toBe( - 'Revert "Revert "Revert "feat: add feature"""' - ); + expect( + extractRevertedTitle( + 'Revert "Revert "Revert "Revert "feat: add feature""""', + ), + ).toBe('Revert "Revert "Revert "feat: add feature"""'); }); it('extracts title with quotes in the message', () => { // Revert "fix: handle "special" case" expect(extractRevertedTitle('Revert "fix: handle "special" case"')).toBe( - 'fix: handle "special" case' + 'fix: handle "special" case', ); }); it('extracts title from double revert with PR suffix', () => { - expect(extractRevertedTitle('Revert "Revert "feat: add feature"" (#456)')).toBe( - 'Revert "feat: add feature"' - ); + expect( + extractRevertedTitle('Revert "Revert "feat: add feature"" (#456)'), + ).toBe('Revert "feat: add feature"'); }); it('returns null for non-revert titles', () => { @@ -400,7 +424,7 @@ describe('processReverts', () => { title: string, body = '', prTitle?: string, - prBody?: string + prBody?: string, ) => ({ hash, title, @@ -417,7 +441,11 @@ describe('processReverts', () => { it('cancels out revert and original via SHA', () => { // Commits in newest-first order (git log order) const commits = [ - commit('def456', 'Revert "feat: add feature"', 'This reverts commit abc123.'), + commit( + 'def456', + 'Revert "feat: add feature"', + 'This reverts commit abc123.', + ), commit('abc123', 'feat: add feature'), ]; const result = processReverts(commits); @@ -436,7 +464,11 @@ describe('processReverts', () => { it('keeps standalone revert when original not in list', () => { const commits = [ - commit('def456', 'Revert "feat: old feature"', 'This reverts commit oldsha.'), + commit( + 'def456', + 'Revert "feat: old feature"', + 'This reverts commit oldsha.', + ), ]; const result = processReverts(commits); expect(result).toHaveLength(1); @@ -446,3 +478,43 @@ describe('processReverts', () => { // Note: "current PR preview scenario" tests are covered by integration tests // in changelog-generate.test.ts under generateChangelogWithHighlight }); + +describe('getNormalizedReleaseConfig semver warnings', () => { + // These tests need to be in a separate file with proper module mocking + // For now, we test the warning behavior through integration with the real config + + beforeEach(() => { + // Clear the memoization cache and mocks before each test + clearReleaseConfigCache(); + vi.clearAllMocks(); + }); + + it('does not warn when using default config', async () => { + const { logger } = await import('../../logger'); + + // Uses real readReleaseConfig which returns defaults when no file exists + getNormalizedReleaseConfig(); + + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('memoizes the result', async () => { + // Call multiple times, should return same object + const result1 = getNormalizedReleaseConfig(); + const result2 = getNormalizedReleaseConfig(); + const result3 = getNormalizedReleaseConfig(); + + expect(result1).toBe(result2); + expect(result2).toBe(result3); + }); + + it('returns fresh result after clearing cache', () => { + const result1 = getNormalizedReleaseConfig(); + clearReleaseConfigCache(); + const result2 = getNormalizedReleaseConfig(); + + // Should be equal but not the same object + expect(result1).not.toBe(result2); + expect(result1).toEqual(result2); + }); +}); diff --git a/src/utils/changelog.ts b/src/utils/changelog.ts index de1c961b..f73c4b48 100644 --- a/src/utils/changelog.ts +++ b/src/utils/changelog.ts @@ -88,6 +88,7 @@ export function isBumpType(value: string): value is BumpType { * Path to the changelog file in the target repository */ export const DEFAULT_CHANGELOG_PATH = 'CHANGELOG.md'; + export const DEFAULT_UNRELEASED_TITLE = 'Unreleased'; export const SKIP_CHANGELOG_MAGIC_WORD = '#skip-changelog'; export const BODY_IN_CHANGELOG_MAGIC_WORD = '#body-in-changelog'; @@ -247,7 +248,9 @@ export interface ChangelogEntryItem { * @param prBody The PR description/body text * @returns Array of changelog entry items, or null if no "Changelog Entry" section is found */ -export function extractChangelogEntry(prBody: string | null | undefined): ChangelogEntryItem[] | null { +export function extractChangelogEntry( + prBody: string | null | undefined, +): ChangelogEntryItem[] | null { if (!prBody) { return null; } @@ -260,7 +263,7 @@ export function extractChangelogEntry(prBody: string | null | undefined): Change (t): t is Tokens.Heading => t.type === 'heading' && (t.depth === 2 || t.depth === 3) && - t.text.toLowerCase() === 'changelog entry' + t.text.toLowerCase() === 'changelog entry', ); if (headingIndex === -1) { @@ -274,7 +277,10 @@ export function extractChangelogEntry(prBody: string | null | undefined): Change for (let i = headingIndex + 1; i < tokens.length; i++) { const token = tokens[i]; // Stop at next heading of same or higher level - if (token.type === 'heading' && (token as Tokens.Heading).depth <= headingDepth) { + if ( + token.type === 'heading' && + (token as Tokens.Heading).depth <= headingDepth + ) { break; } contentTokens.push(token); @@ -326,7 +332,9 @@ function extractNestedContent(tokens: Token[]): string { function getListItemText(item: Tokens.ListItem): string { // The item.text contains the raw text, but we want just the first line // (before any nested lists) - const firstToken = item.tokens.find(t => t.type === 'text' || t.type === 'paragraph'); + const firstToken = item.tokens.find( + t => t.type === 'text' || t.type === 'paragraph', + ); if (firstToken && 'text' in firstToken) { return firstToken.text.split('\n')[0].trim(); } @@ -396,7 +404,7 @@ function extractChangeset(markdown: string, location: ChangesetLoc): Changeset { */ function locateChangeset( markdown: string, - predicate: (match: string) => boolean + predicate: (match: string) => boolean, ): ChangesetLoc | null { const tokens = marked.lexer(markdown); @@ -462,7 +470,7 @@ function locateChangeset( export function findChangeset( markdown: string, tag: string, - fallbackToUnreleased = false + fallbackToUnreleased = false, ): Changeset | null { const version = getVersion(tag); if (version === null) { @@ -471,12 +479,12 @@ export function findChangeset( let changesetLoc = locateChangeset( markdown, - match => getVersion(match) === version + match => getVersion(match) === version, ); if (!changesetLoc && fallbackToUnreleased) { changesetLoc = locateChangeset( markdown, - match => match === DEFAULT_UNRELEASED_TITLE + match => match === DEFAULT_UNRELEASED_TITLE, ); } @@ -495,7 +503,9 @@ export function removeChangeset(markdown: string, header: string): string { return markdown; } - return markdown.slice(0, location.startIndex) + markdown.slice(location.endIndex); + return ( + markdown.slice(0, location.startIndex) + markdown.slice(location.endIndex) + ); } /** @@ -511,7 +521,7 @@ export function removeChangeset(markdown: string, header: string): string { */ export function prependChangeset( markdown: string, - changeset: Changeset + changeset: Changeset, ): string { // Try to locate the top-most non-empty header, no matter what is inside const firstHeading = locateChangeset(markdown, Boolean); @@ -560,7 +570,7 @@ function createPREntriesFromRaw( highlight?: boolean; }, defaultTitle: string, - fallbackBody?: string + fallbackBody?: string, ): PullRequest[] { const customEntries = extractChangelogEntry(raw.prBody); @@ -729,7 +739,7 @@ export function processReverts(rawCommits: RawCommitInfo[]): RawCommitInfo[] { remaining.delete(commit); remaining.delete(revertedCommit); logger.debug( - `Revert cancellation: "${effectiveTitle}" cancels "${(revertedCommit.prTitle ?? revertedCommit.title).trim()}"` + `Revert cancellation: "${effectiveTitle}" cancels "${(revertedCommit.prTitle ?? revertedCommit.title).trim()}"`, ); } } @@ -849,39 +859,48 @@ type CategoryWithPRs = { scopeGroups: Map; }; +/** Result of reading release config, including whether it's a custom config */ +export interface ReleaseConfigResult { + config: ReleaseConfig; + /** True if a custom .github/release.yml was loaded (not using defaults) */ + isCustomConfig: boolean; +} + /** * Reads and parses .github/release.yml from the repository root - * @returns Parsed release configuration, or the default config if file doesn't exist + * @returns Parsed release configuration with metadata about source */ -export function readReleaseConfig(): ReleaseConfig { +export function readReleaseConfig(): ReleaseConfigResult { const configFileDir = getConfigFileDir(); if (!configFileDir) { - return DEFAULT_RELEASE_CONFIG; + return { config: DEFAULT_RELEASE_CONFIG, isCustomConfig: false }; } const releaseConfigPath = join(configFileDir, '.github', 'release.yml'); try { const fileContents = readFileSync(releaseConfigPath, 'utf8'); const config = load(fileContents) as ReleaseConfig; - return config; + return { config, isCustomConfig: true }; } catch (error: any) { if (error.code === 'ENOENT') { // File doesn't exist, return default config - return DEFAULT_RELEASE_CONFIG; + return { config: DEFAULT_RELEASE_CONFIG, isCustomConfig: false }; } logger.warn( `Failed to read release config from ${releaseConfigPath}:`, - error + error, ); - return DEFAULT_RELEASE_CONFIG; + return { config: DEFAULT_RELEASE_CONFIG, isCustomConfig: false }; } } /** - * Normalizes the release config by converting arrays to Sets and compiling regex patterns + * Normalizes the release config by converting arrays to Sets and compiling regex patterns. + * + * @param config The raw release configuration */ export function normalizeReleaseConfig( - config: ReleaseConfig + config: ReleaseConfig, ): NormalizedReleaseConfig | null { if (!config?.changelog) { return null; @@ -903,7 +922,7 @@ export function normalizeReleaseConfig( config.changelog.exclude.labels.length > 0 ) { normalized.changelog.exclude.labels = new Set( - config.changelog.exclude.labels + config.changelog.exclude.labels, ); } if ( @@ -911,7 +930,7 @@ export function normalizeReleaseConfig( config.changelog.exclude.authors.length > 0 ) { normalized.changelog.exclude.authors = new Set( - config.changelog.exclude.authors + config.changelog.exclude.authors, ); } } @@ -931,7 +950,7 @@ export function normalizeReleaseConfig( return new RegExp(pattern, 'i'); } catch { logger.warn( - `Invalid regex pattern in release config: ${pattern}` + `Invalid regex pattern in release config: ${pattern}`, ); return null; } @@ -947,24 +966,67 @@ export function normalizeReleaseConfig( if (category.exclude) { if (category.exclude.labels && category.exclude.labels.length > 0) { normalizedCategory.exclude.labels = new Set( - category.exclude.labels + category.exclude.labels, ); } if (category.exclude.authors && category.exclude.authors.length > 0) { normalizedCategory.exclude.authors = new Set( - category.exclude.authors + category.exclude.authors, ); } } return normalizedCategory; - } + }, ); } return normalized; } +// Memoization cache for getNormalizedReleaseConfig +let releaseConfigCache: NormalizedReleaseConfig | null | undefined; + +/** + * Clears the release config cache. Primarily used for testing. + */ +export function clearReleaseConfigCache(): void { + releaseConfigCache = undefined; +} + +/** + * Reads, normalizes, and caches the release configuration. + * Warns once if a custom config has categories without semver fields. + * + * @returns Normalized release configuration, or null if invalid + */ +export function getNormalizedReleaseConfig(): NormalizedReleaseConfig | null { + if (releaseConfigCache !== undefined) { + return releaseConfigCache; + } + + const { config, isCustomConfig } = readReleaseConfig(); + const normalized = normalizeReleaseConfig(config); + + // Warn about missing semver fields in custom configs + if (isCustomConfig && normalized) { + const categoriesWithoutSemver = normalized.changelog.categories + .filter(cat => !cat.semver) + .map(cat => cat.title); + + if (categoriesWithoutSemver.length > 0) { + logger.warn( + `The following changelog categories have no 'semver' field and won't contribute to ` + + `version bump detection: ${categoriesWithoutSemver.map(t => `"${t}"`).join(', ')}. ` + + `See: https://getsentry.github.io/craft/configuration/#auto-mode`, + ); + } + } + + releaseConfigCache = normalized; + return normalized; +} + /** * Checks if a PR should be excluded globally based on: * 1. The #skip-changelog magic word in the body (commit body or PR body) @@ -981,7 +1043,7 @@ export function shouldExcludePR( labels: Set, author: string | undefined, config: NormalizedReleaseConfig | null, - body?: string + body?: string, ): boolean { // Check for magic word in body if (body?.includes(SKIP_CHANGELOG_MAGIC_WORD)) { @@ -1015,8 +1077,7 @@ export function shouldExcludePR( * @returns true if the PR should be skipped */ export function shouldSkipCurrentPR(prInfo: CurrentPRInfo): boolean { - const rawConfig = readReleaseConfig(); - const releaseConfig = normalizeReleaseConfig(rawConfig); + const releaseConfig = getNormalizedReleaseConfig(); const labels = new Set(prInfo.labels); return shouldExcludePR(labels, prInfo.author, releaseConfig, prInfo.body); @@ -1031,15 +1092,14 @@ export function shouldSkipCurrentPR(prInfo: CurrentPRInfo): boolean { * @returns The bump type (major, minor, patch) or null if no match */ export function getBumpTypeForPR(prInfo: CurrentPRInfo): BumpType | null { - const rawConfig = readReleaseConfig(); - const releaseConfig = normalizeReleaseConfig(rawConfig); + const releaseConfig = getNormalizedReleaseConfig(); const labels = new Set(prInfo.labels); const match = matchCommitToCategory( labels, prInfo.author, prInfo.title.trim(), - releaseConfig + releaseConfig, ); return match?.category.semver ?? null; @@ -1051,7 +1111,7 @@ export function getBumpTypeForPR(prInfo: CurrentPRInfo): BumpType | null { export function isCategoryExcluded( category: NormalizedCategory, labels: Set, - author: string | undefined + author: string | undefined, ): boolean { if (labels.size > 0) { for (const excludeLabel of category.exclude.labels) { @@ -1088,7 +1148,7 @@ export function matchCommitToCategory( labels: Set, author: string | undefined, title: string, - config: NormalizedReleaseConfig | null + config: NormalizedReleaseConfig | null, ): CategoryMatchResult | null { if (!config?.changelog || config.changelog.categories.length === 0) { return null; @@ -1179,7 +1239,7 @@ interface ChangelogEntry { export function stripTitle( title: string, pattern: RegExp | undefined, - preserveScope: boolean + preserveScope: boolean, ): string { if (!pattern) return title; @@ -1208,7 +1268,7 @@ export function stripTitle( function formatChangelogEntry( entry: ChangelogEntry, disableMentions = false, - disablePRLinks = false + disablePRLinks = false, ): string { let title = entry.title; @@ -1352,7 +1412,7 @@ export function clearChangesetCache(): void { export async function generateChangesetFromGit( git: SimpleGit, rev: string, - maxLeftovers: number = MAX_LEFTOVERS + maxLeftovers: number = MAX_LEFTOVERS, ): Promise { const cacheKey = getChangesetCacheKey(rev, maxLeftovers); @@ -1386,7 +1446,7 @@ export async function generateChangesetFromGit( export async function generateChangelogWithHighlight( git: SimpleGit, rev: string, - currentPRNumber: number + currentPRNumber: number, ): Promise { // Step 1: Fetch PR info from GitHub const prInfo = await fetchPRInfo(currentPRNumber); @@ -1447,7 +1507,12 @@ export async function generateChangelogWithHighlight( const { data: rawData, stats } = categorizeCommits(filteredCommits); // Step 10: Serialize to markdown (disable mentions and PR links to avoid pinging/cross-referencing in PR preview comments) - const changelog = await serializeChangelog(rawData, MAX_LEFTOVERS, true, true); + const changelog = await serializeChangelog( + rawData, + MAX_LEFTOVERS, + true, + true, + ); // Determine bump type: // - If current PR survived revert processing, use its specific bump type (PR 676 fix) @@ -1477,15 +1542,15 @@ export async function generateChangelogWithHighlight( async function fetchRawCommitInfo( git: SimpleGit, rev: string, - until?: string + until?: string, ): Promise { // Early filter: skip commits with magic word in commit body (optimization to avoid GitHub API calls) const gitCommits = (await getChangesSince(git, rev, until)).filter( - ({ body }) => !body.includes(SKIP_CHANGELOG_MAGIC_WORD) + ({ body }) => !body.includes(SKIP_CHANGELOG_MAGIC_WORD), ); const githubCommits = await getPRAndLabelsFromCommit( - gitCommits.map(({ hash }) => hash) + gitCommits.map(({ hash }) => hash), ); // Build commit list with deduplication by PR number in a single pass. @@ -1530,8 +1595,7 @@ async function fetchRawCommitInfo( * @returns Categorized changelog data and stats */ function categorizeCommits(rawCommits: RawCommitInfo[]): RawChangelogResult { - const rawConfig = readReleaseConfig(); - const releaseConfig = normalizeReleaseConfig(rawConfig); + const releaseConfig = getNormalizedReleaseConfig(); const categories = new Map(); const leftovers: Commit[] = []; @@ -1556,7 +1620,7 @@ function categorizeCommits(rawCommits: RawCommitInfo[]): RawChangelogResult { labels, raw.author, titleForMatching, - releaseConfig + releaseConfig, ); const matchedCategory = match?.category ?? null; const matchedPattern = match?.matchedPattern; @@ -1635,7 +1699,7 @@ function categorizeCommits(rawCommits: RawCommitInfo[]): RawChangelogResult { if (missing.length > 0) { logger.warn( 'The following commits were not found on GitHub:', - missing.map(c => `${c.hash.slice(0, 8)} ${c.title}`) + missing.map(c => `${c.hash.slice(0, 8)} ${c.title}`), ); } @@ -1665,7 +1729,7 @@ function categorizeCommits(rawCommits: RawCommitInfo[]): RawChangelogResult { async function generateRawChangelog( git: SimpleGit, rev: string, - until?: string + until?: string, ): Promise { const rawCommits = await fetchRawCommitInfo(git, rev, until); // Process reverts: cancel out revert/reverted pairs @@ -1687,7 +1751,7 @@ async function serializeChangelog( rawData: RawChangelogData, maxLeftovers: number, disableMentions = false, - disablePRLinks = false + disablePRLinks = false, ): Promise { const { categories, leftovers, releaseConfig } = rawData; @@ -1723,7 +1787,7 @@ async function serializeChangelog( } changelogSections.push( - markdownHeader(SUBSECTION_HEADER_LEVEL, category.title) + markdownHeader(SUBSECTION_HEADER_LEVEL, category.title), ); // Sort scope groups: scoped entries first (alphabetically), scopeless (null) last @@ -1740,7 +1804,7 @@ async function serializeChangelog( // Check if any scope has multiple entries (would get its own header) const hasScopeHeaders = [...category.scopeGroups.entries()].some( - ([s, entries]) => s !== null && entries.length > 1 + ([s, entries]) => s !== null && entries.length > 1, ); // Collect entries without headers to combine them into a single section @@ -1774,8 +1838,8 @@ async function serializeChangelog( highlight: pr.highlight, }, disableMentions, - disablePRLinks - ) + disablePRLinks, + ), ); if (scopeHeader) { @@ -1815,7 +1879,7 @@ async function serializeChangelog( highlight: commit.highlight, }, (commit.prTitle ?? commit.title).trim(), - commit.body // fallback for magic word check + commit.body, // fallback for magic word check ); for (const pr of prEntries) { @@ -1831,8 +1895,8 @@ async function serializeChangelog( highlight: pr.highlight, }, disableMentions, - disablePRLinks - ) + disablePRLinks, + ), ); } } @@ -1852,7 +1916,7 @@ async function serializeChangelog( async function generateChangesetFromGitImpl( git: SimpleGit, rev: string, - maxLeftovers: number + maxLeftovers: number, ): Promise { const { data: rawData, stats } = await generateRawChangelog(git, rev); const changelog = await serializeChangelog(rawData, maxLeftovers); @@ -1915,7 +1979,7 @@ export async function getPRAndLabelsFromCommit(hashes: string[]): Promise< for (let chunk = 0; chunk < chunkCount; chunk += 1) { const subset = hashes.slice( chunk * MAX_COMMITS_PER_QUERY, - (chunk + 1) * MAX_COMMITS_PER_QUERY + (chunk + 1) * MAX_COMMITS_PER_QUERY, ); const commitsQuery = subset @@ -1984,7 +2048,7 @@ export async function getPRAndLabelsFromCommit(hashes: string[]): Promise< logger.warn( `Failed to fetch PR info for chunk ${chunk + 1}/${chunkCount} ` + `(${subset.length} commits): ${error.message || error}. ` + - `Skipping this chunk and continuing with remaining commits.` + `Skipping this chunk and continuing with remaining commits.`, ); for (const hash of subset) { @@ -2015,6 +2079,6 @@ export async function getPRAndLabelsFromCommit(hashes: string[]): Promise< labels: [], }, ]; - }) + }), ); }