From 72d40df677b08fd52654a4c3b320649f63b9c635 Mon Sep 17 00:00:00 2001 From: Chris Date: Wed, 13 Nov 2024 00:00:15 +0100 Subject: [PATCH] feat: always-update (#2420) * feat: add "always-update" config option Fixes #1459 This patch adds a new option "always-update" that forces existing pull requests to be updated even when there are no changes to the release notes. This is useful, for example, when the repository enforces pull requests to be up-to-date with the main branch before they can be merged. Without this option set to `true`, that is, with the default behavior of release-please, if the last commit made to the main branch does not appear in the release notes, the existing pull request branch would not be rebased. * feat: apply suggested changes --------- Co-authored-by: Fabian Meyer <3982806+meyfa@users.noreply.github.com> Co-authored-by: Jeff Ching --- docs/manifest-releaser.md | 7 ++++ schemas/config.json | 5 +++ src/manifest.ts | 50 ++++++++++++++--------- test/manifest.ts | 83 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 126 insertions(+), 19 deletions(-) diff --git a/docs/manifest-releaser.md b/docs/manifest-releaser.md index 92e24a273..0bf9c0880 100644 --- a/docs/manifest-releaser.md +++ b/docs/manifest-releaser.md @@ -218,6 +218,13 @@ defaults (those are documented in comments) // absence defaults to false and one pull request will be raised "separate-pull-requests": false, + // if true, always update existing pull requests when changes are added, + // instead of only when the release notes change. + // This option may increase the number of API calls used, but can be useful + // if pull requests must not be out-of-date with the base branch. + // absence defaults to false + "always-update": true, + // sets the manifest pull request title for when releasing multiple packages // grouped together in the one pull request. // This option has no effect when `separate-pull-requests` is `true`. diff --git a/schemas/config.json b/schemas/config.json index 3fc917901..9d883effa 100644 --- a/schemas/config.json +++ b/schemas/config.json @@ -111,6 +111,10 @@ "description": "Open a separate release pull request for each component. Defaults to `false`.", "type": "boolean" }, + "always-update": { + "description": "Always update the pull request with the latest changes. Defaults to `false`.", + "type": "boolean" + }, "tag-separator": { "description": "Customize the separator between the component and version in the GitHub tag.", "type": "string" @@ -470,6 +474,7 @@ "pull-request-header": true, "pull-request-footer": true, "separate-pull-requests": true, + "always-update": true, "tag-separator": true, "extra-files": true, "version-file": true, diff --git a/src/manifest.ts b/src/manifest.ts index 13e590704..390154573 100644 --- a/src/manifest.ts +++ b/src/manifest.ts @@ -202,6 +202,7 @@ export interface ManifestOptions { draft?: boolean; prerelease?: boolean; draftPullRequest?: boolean; + alwaysUpdate?: boolean; groupPullRequestTitlePattern?: string; releaseSearchDepth?: number; commitSearchDepth?: number; @@ -260,6 +261,7 @@ export interface ManifestConfig extends ReleaserConfigJson { 'release-search-depth'?: number; 'commit-search-depth'?: number; 'sequential-calls'?: boolean; + 'always-update'?: boolean; } // path => version export type ReleasedVersions = Record; @@ -295,6 +297,7 @@ export class Manifest { readonly releasedVersions: ReleasedVersions; private targetBranch: string; private separatePullRequests: boolean; + private alwaysUpdate: boolean; readonly fork: boolean; private signoffUser?: string; private labels: string[]; @@ -334,6 +337,8 @@ export class Manifest { * plugin * @param {boolean} manifestOptions.separatePullRequests If true, create separate pull * requests instead of a single manifest release pull request + * @param {boolean} manifestOptions.alwaysUpdate If true, always updates pull requests instead of + * only when the release notes change * @param {PluginType[]} manifestOptions.plugins Any plugins to use for this repository * @param {boolean} manifestOptions.fork If true, create pull requests from a fork. Defaults * to `false` @@ -361,6 +366,7 @@ export class Manifest { this.separatePullRequests = manifestOptions?.separatePullRequests ?? Object.keys(repositoryConfig).length === 1; + this.alwaysUpdate = manifestOptions?.alwaysUpdate || false; this.fork = manifestOptions?.fork || false; this.signoffUser = manifestOptions?.signoff; this.releaseLabels = @@ -1016,7 +1022,9 @@ export class Manifest { openPullRequest.headBranchName === pullRequest.headRefName ); if (existing) { - return await this.maybeUpdateExistingPullRequest(existing, pullRequest); + return this.alwaysUpdate + ? await this.updateExistingPullRequest(existing, pullRequest) + : await this.maybeUpdateExistingPullRequest(existing, pullRequest); } // look for closed, snoozed pull request @@ -1025,7 +1033,9 @@ export class Manifest { openPullRequest.headBranchName === pullRequest.headRefName ); if (snoozed) { - return await this.maybeUpdateSnoozedPullRequest(snoozed, pullRequest); + return this.alwaysUpdate + ? await this.updateExistingPullRequest(snoozed, pullRequest) + : await this.maybeUpdateSnoozedPullRequest(snoozed, pullRequest); } const body = await this.pullRequestOverflowHandler.handleOverflow( @@ -1068,20 +1078,10 @@ export class Manifest { ); return undefined; } - const updatedPullRequest = await this.github.updatePullRequest( - existing.number, - pullRequest, - this.targetBranch, - { - fork: this.fork, - signoffUser: this.signoffUser, - pullRequestOverflowHandler: this.pullRequestOverflowHandler, - } - ); - return updatedPullRequest; + return await this.updateExistingPullRequest(existing, pullRequest); } - /// only update an snoozed pull request if it has release note changes + /// only update a snoozed pull request if it has release note changes private async maybeUpdateSnoozedPullRequest( snoozed: PullRequest, pullRequest: ReleasePullRequest @@ -1093,8 +1093,22 @@ export class Manifest { ); return undefined; } - const updatedPullRequest = await this.github.updatePullRequest( - snoozed.number, + const updatedPullRequest = await this.updateExistingPullRequest( + snoozed, + pullRequest + ); + // TODO: consider leaving the snooze label + await this.github.removeIssueLabels([SNOOZE_LABEL], snoozed.number); + return updatedPullRequest; + } + + /// force an update to an existing pull request + private async updateExistingPullRequest( + existing: PullRequest, + pullRequest: ReleasePullRequest + ): Promise { + return await this.github.updatePullRequest( + existing.number, pullRequest, this.targetBranch, { @@ -1103,9 +1117,6 @@ export class Manifest { pullRequestOverflowHandler: this.pullRequestOverflowHandler, } ); - // TODO: consider leaving the snooze label - await this.github.removeIssueLabels([SNOOZE_LABEL], snoozed.number); - return updatedPullRequest; } private async *findMergedReleasePullRequests() { @@ -1428,6 +1439,7 @@ async function parseConfig( lastReleaseSha: config['last-release-sha'], alwaysLinkLocal: config['always-link-local'], separatePullRequests: config['separate-pull-requests'], + alwaysUpdate: config['always-update'], groupPullRequestTitlePattern: config['group-pull-request-title-pattern'], plugins: config['plugins'], signoff: config['signoff'], diff --git a/test/manifest.ts b/test/manifest.ts index f8394290a..9c0d44caf 100644 --- a/test/manifest.ts +++ b/test/manifest.ts @@ -4330,6 +4330,89 @@ describe('Manifest', () => { const pullRequestNumbers = await manifest.createPullRequests(); expect(pullRequestNumbers).lengthOf(0); }); + + it('updates an existing pull request without changes if set to always update', async function () { + sandbox + .stub(github, 'getFileContentsOnBranch') + .withArgs('README.md', 'main') + .resolves(buildGitHubFileRaw('some-content')) + .withArgs('release-notes.md', 'my-head-branch--release-notes') + .resolves(buildGitHubFileRaw(body.toString())); + stubSuggesterWithSnapshot(sandbox, this.test!.fullTitle()); + mockPullRequests( + github, + [ + { + number: 22, + title: 'pr title1', + body: pullRequestBody('release-notes/overflow.txt'), + headBranchName: 'release-please/branches/main', + baseBranchName: 'main', + labels: ['autorelease: pending'], + files: [], + }, + ], + [] + ); + sandbox + .stub(github, 'updatePullRequest') + .withArgs( + 22, + sinon.match.any, + sinon.match.any, + sinon.match.has('pullRequestOverflowHandler', sinon.match.truthy) + ) + .resolves({ + number: 22, + title: 'pr title1', + body: 'pr body1', + headBranchName: 'release-please/branches/main', + baseBranchName: 'main', + labels: [], + files: [], + }); + const manifest = new Manifest( + github, + 'main', + { + 'path/a': { + releaseType: 'node', + component: 'pkg1', + }, + 'path/b': { + releaseType: 'node', + component: 'pkg2', + }, + }, + { + 'path/a': Version.parse('1.0.0'), + 'path/b': Version.parse('0.2.3'), + }, + { + separatePullRequests: true, + alwaysUpdate: true, + plugins: ['node-workspace'], + } + ); + sandbox.stub(manifest, 'buildPullRequests').resolves([ + { + title: PullRequestTitle.ofTargetBranch('main'), + body, + updates: [ + { + path: 'README.md', + createIfMissing: false, + updater: new RawContent('some raw content'), + }, + ], + labels: [], + headRefName: 'release-please/branches/main', + draft: false, + }, + ]); + const pullRequestNumbers = await manifest.createPullRequests(); + expect(pullRequestNumbers).lengthOf(1); + }); }); it('updates an existing snapshot pull request', async function () {