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: [], }, ]; - }) + }), ); }