diff --git a/src/manifest.ts b/src/manifest.ts index 2402335fe..934c1d9ae 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -298,7 +298,7 @@ export class Manifest { const latestVersion = await latestReleaseVersion( github, targetBranch, - component + config.includeComponentInTag ? component : '' ); if (latestVersion) { releasedVersions[path] = latestVersion; @@ -789,8 +789,7 @@ export class Manifest { const strategiesByPath = await this.getStrategiesByPath(); for (const path in this.repositoryConfig) { const strategy = strategiesByPath[path]; - const component = - strategy.component || (await strategy.getDefaultComponent()) || ''; + const component = (await strategy.getComponent()) || ''; if (this._pathsByComponent[component]) { logger.warn( `Multiple paths for ${component}: ${this._pathsByComponent[component]}, ${path}` diff --git a/src/strategy.ts b/src/strategy.ts index a18bc11af..e024ab0d7 100644 --- a/src/strategy.ts +++ b/src/strategy.ts @@ -115,6 +115,9 @@ export abstract class Strategy { ): Promise; async getComponent(): Promise { + if (!this.includeComponentInTag) { + return ''; + } return this.component || (await this.getDefaultComponent()); } @@ -290,7 +293,12 @@ export abstract class Strategy { mergedPullRequest: PullRequest ): Promise { if (this.skipGitHubRelease) { - return undefined; + logger.info('Release skipped from strategy config'); + return; + } + if (!mergedPullRequest.sha) { + logger.error('Pull request should have been merged'); + return; } const pullRequestTitle = @@ -300,18 +308,18 @@ export abstract class Strategy { MANIFEST_PULL_REQUEST_TITLE_PATTERN ); if (!pullRequestTitle) { - throw new Error(`Bad pull request title: '${mergedPullRequest.title}'`); + logger.error(`Bad pull request title: '${mergedPullRequest.title}'`); + return; } const branchName = BranchName.parse(mergedPullRequest.headBranchName); if (!branchName) { - throw new Error(`Bad branch name: ${mergedPullRequest.headBranchName}`); - } - if (!mergedPullRequest.sha) { - throw new Error('Pull request should have been merged'); + logger.error(`Bad branch name: ${mergedPullRequest.headBranchName}`); + return; } const pullRequestBody = PullRequestBody.parse(mergedPullRequest.body); if (!pullRequestBody) { - throw new Error('could not parse pull request body as a release PR'); + logger.error('Could not parse pull request body as a release PR'); + return; } const component = await this.getComponent(); logger.info('component:', component); @@ -331,7 +339,8 @@ export abstract class Strategy { } const version = pullRequestTitle.getVersion() || releaseData?.version; if (!version) { - throw new Error('Pull request should have included version'); + logger.error('Pull request should have included version'); + return; } const tag = new TagName( diff --git a/test/manifest.ts b/test/manifest.ts index b60fb894d..a68e0d331 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -180,6 +180,62 @@ describe('Manifest', () => { expect(Object.keys(manifest.repositoryConfig)).lengthOf(1); expect(Object.keys(manifest.releasedVersions)).lengthOf(1); }); + it('finds previous release without tag', async () => { + mockCommits(github, [ + { + sha: 'abc123', + message: 'some commit message', + files: [], + pullRequest: { + headBranchName: 'release-please/branches/main', + baseBranchName: 'main', + number: 123, + title: 'chore: release 1.2.3', + body: '', + labels: [], + files: [], + }, + }, + ]); + + const manifest = await Manifest.fromConfig(github, 'target-branch', { + releaseType: 'simple', + bumpMinorPreMajor: true, + bumpPatchForMinorPreMajor: true, + component: 'foobar', + includeComponentInTag: false, + }); + expect(Object.keys(manifest.repositoryConfig)).lengthOf(1); + expect(Object.keys(manifest.releasedVersions)).lengthOf(1); + }); + it('finds previous release with tag', async () => { + mockCommits(github, [ + { + sha: 'abc123', + message: 'some commit message', + files: [], + pullRequest: { + headBranchName: 'release-please/branches/main/components/foobar', + baseBranchName: 'main', + number: 123, + title: 'chore: release foobar 1.2.3', + body: '', + labels: [], + files: [], + }, + }, + ]); + + const manifest = await Manifest.fromConfig(github, 'target-branch', { + releaseType: 'simple', + bumpMinorPreMajor: true, + bumpPatchForMinorPreMajor: true, + component: 'foobar', + includeComponentInTag: true, + }); + expect(Object.keys(manifest.repositoryConfig)).lengthOf(1); + expect(Object.keys(manifest.releasedVersions)).lengthOf(1); + }); }); describe('buildPullRequests', () => { @@ -1876,8 +1932,8 @@ describe('Manifest', () => { headBranchName: 'release-please/branches/main', baseBranchName: 'main', number: 1234, - title: 'chore: release main', - body: pullRequestBody('release-notes/single-manifest.txt'), + title: 'chore(main): release v1.3.1', + body: pullRequestBody('release-notes/single.txt'), labels: ['autorelease: pending'], files: [], sha: 'abc123',