From 62d678d1478bf477b097a58c2bb454b7e1dcc228 Mon Sep 17 00:00:00 2001 From: Mike Heffner <mikeh@fesnel.com> Date: Fri, 1 Mar 2024 13:43:07 -0500 Subject: [PATCH] feat: Add new keepUpdatedLabel config setting (#27542) Co-authored-by: Rhys Arkins <rhys@arkins.net> --- docs/usage/configuration-options.md | 9 ++ docs/usage/updating-rebasing.md | 1 + lib/config/options/index.ts | 7 ++ lib/config/types.ts | 1 + .../repository/update/branch/index.spec.ts | 83 +++++++++++++++++++ lib/workers/repository/update/branch/index.ts | 5 +- .../repository/update/branch/reuse.spec.ts | 33 ++++++++ lib/workers/repository/update/branch/reuse.ts | 28 ++++++- 8 files changed, 165 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration-options.md b/docs/usage/configuration-options.md index ce0c1d80b2..07e9112da8 100644 --- a/docs/usage/configuration-options.md +++ b/docs/usage/configuration-options.md @@ -2123,6 +2123,15 @@ Currently this applies to the `minimumReleaseAge` check only. The `flexible` mode can result in "flapping" of Pull Requests, for example: a pending PR with version `1.0.3` is first released but then downgraded to `1.0.2` once it passes `minimumReleaseAge`. We recommend that you use the `strict` mode, and enable the `dependencyDashboard` so that you can see suppressed PRs. +## keepUpdatedLabel + +On supported platforms it is possible to add a label to a PR to recreate/rebase it when the branch falls 1 or more commits behind its base branch. +Adding the `keepUpdatedLabel` label to a PR makes Renovate behave as if `rebaseWhen` were set to `behind-base-branch`, but only for the given PR. +The label is not removed from the PR after the rebase is complete, unlike what happens with `rebaseLabel`. + +This can be useful when you have approved certain PRs and want to keep them updated until they are ready to be merged. +The setting `keepUpdatedLabel` is best used in conjunction with `rebaseWhen` set to the values of `never` or `conflicted` that limit rebasing. + ## labels By default, Renovate won't add any labels to PRs. diff --git a/docs/usage/updating-rebasing.md b/docs/usage/updating-rebasing.md index 6bfadb298f..67d50660ed 100644 --- a/docs/usage/updating-rebasing.md +++ b/docs/usage/updating-rebasing.md @@ -12,6 +12,7 @@ Here is a list of the most common cases where Renovate must update/rebase the br - When a pull request has conflicts due to changes on the base branch - When you have enabled "Require branches to be up to date before merging" on GitHub - When you have manually told Renovate to rebase when behind the base branch with `"rebaseWhen": "behind-base-branch"` +- When you have set `keepUpdatedLabel` and included the label on a PR - When a newer version of the dependency is released - When you request a manual rebase from the Renovate bot - When you use `"automerge": true` and `"rebaseWhen": "auto"` on a branch / pr diff --git a/lib/config/options/index.ts b/lib/config/options/index.ts index 4b441f962d..f2b509c9a5 100644 --- a/lib/config/options/index.ts +++ b/lib/config/options/index.ts @@ -1752,6 +1752,13 @@ const options: RenovateOptions[] = [ default: 'auto', }, // PR Behaviour + { + name: 'keepUpdatedLabel', + description: + 'Label to request Renovate bot always rebase to keep branch updated.', + type: 'string', + supportedPlatforms: ['azure', 'gitea', 'github', 'gitlab'], + }, { name: 'rollbackPrs', description: diff --git a/lib/config/types.ts b/lib/config/types.ts index 6e0715dabd..ac5935ed99 100644 --- a/lib/config/types.ts +++ b/lib/config/types.ts @@ -58,6 +58,7 @@ export interface RenovateSharedConfig { ignorePaths?: string[]; ignoreTests?: boolean; internalChecksAsSuccess?: boolean; + keepUpdatedLabel?: string; labels?: string[]; addLabels?: string[]; dependencyDashboardApproval?: boolean; diff --git a/lib/workers/repository/update/branch/index.spec.ts b/lib/workers/repository/update/branch/index.spec.ts index 5110631355..25e4511fa8 100644 --- a/lib/workers/repository/update/branch/index.spec.ts +++ b/lib/workers/repository/update/branch/index.spec.ts @@ -907,6 +907,35 @@ describe('workers/repository/update/branch/index', () => { expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(1); }); + it('ensures PR when impossible to automerge with mismatch keepUpdatedLabel', async () => { + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce( + partial<PackageFilesResult>({ + updatedPackageFiles: [partial<FileChange>()], + }), + ); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [partial<FileChange>()], + }); + scm.branchExists.mockResolvedValue(true); + automerge.tryBranchAutomerge.mockResolvedValueOnce('stale'); + prWorker.ensurePr.mockResolvedValueOnce({ + type: 'with-pr', + pr: partial<Pr>({ labels: ['keep-not-updated'] }), + }); + prAutomerge.checkAutoMerge.mockResolvedValueOnce({ automerged: false }); + commit.commitFilesToBranch.mockResolvedValueOnce(null); + await branchWorker.processBranch({ + ...config, + automerge: true, + rebaseWhen: 'conflicted', + keepUpdatedLabel: 'keep-updated', + }); + expect(prWorker.ensurePr).toHaveBeenCalledTimes(1); + expect(platform.ensureCommentRemoval).toHaveBeenCalledTimes(0); + expect(prAutomerge.checkAutoMerge).toHaveBeenCalledTimes(1); + }); + it('skips when automerge is off schedule', async () => { getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce( partial<PackageFilesResult>(), @@ -2037,6 +2066,60 @@ describe('workers/repository/update/branch/index', () => { expect(commit.commitFilesToBranch).not.toHaveBeenCalled(); }); + it('continues when rebaseWhen=never and keepUpdatedLabel', async () => { + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ + ...updatedPackageFiles, + }); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + scm.branchExists.mockResolvedValue(true); + platform.getBranchPr.mockResolvedValueOnce( + partial<Pr>({ + state: 'open', + title: '', + labels: ['keep-updated'], + }), + ); + commit.commitFilesToBranch.mockResolvedValueOnce(null); + expect( + await branchWorker.processBranch({ + ...config, + rebaseWhen: 'never', + keepUpdatedLabel: 'keep-updated', + }), + ).toMatchObject({ result: 'done' }); + expect(commit.commitFilesToBranch).toHaveBeenCalled(); + }); + + it('returns when rebaseWhen=never and keepUpdatedLabel does not match', async () => { + getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ + ...updatedPackageFiles, + }); + npmPostExtract.getAdditionalFiles.mockResolvedValueOnce({ + artifactErrors: [], + updatedArtifacts: [], + }); + scm.branchExists.mockResolvedValue(true); + platform.getBranchPr.mockResolvedValueOnce( + partial<Pr>({ + state: 'open', + title: '', + labels: ['keep-updated'], + }), + ); + commit.commitFilesToBranch.mockResolvedValueOnce(null); + expect( + await branchWorker.processBranch({ + ...config, + rebaseWhen: 'never', + keepUpdatedLabel: 'keep-not-updated', + }), + ).toMatchObject({ result: 'no-work' }); + expect(commit.commitFilesToBranch).not.toHaveBeenCalled(); + }); + it('continues when rebaseWhen=never but checked', async () => { getUpdated.getUpdatedPackageFiles.mockResolvedValueOnce({ ...updatedPackageFiles, diff --git a/lib/workers/repository/update/branch/index.ts b/lib/workers/repository/update/branch/index.ts index 050ce7403f..6e23c8f6c7 100644 --- a/lib/workers/repository/update/branch/index.ts +++ b/lib/workers/repository/update/branch/index.ts @@ -145,6 +145,7 @@ export async function processBranch( config.rebaseRequested = await rebaseCheck(config, branchPr); logger.debug(`PR rebase requested=${config.rebaseRequested}`); } + const keepUpdatedLabel = config.keepUpdatedLabel; const artifactErrorTopic = emojify(':warning: Artifact update problem'); try { // Check if branch already existed @@ -429,6 +430,7 @@ export async function processBranch( } else if ( branchExists && config.rebaseWhen === 'never' && + !(keepUpdatedLabel && branchPr?.labels?.includes(keepUpdatedLabel)) && !dependencyDashboardCheck ) { logger.debug('rebaseWhen=never so skipping branch update check'); @@ -636,7 +638,8 @@ export async function processBranch( } if ( mergeStatus === 'stale' && - ['conflicted', 'never'].includes(config.rebaseWhen!) + ['conflicted', 'never'].includes(config.rebaseWhen!) && + !(keepUpdatedLabel && branchPr?.labels?.includes(keepUpdatedLabel)) ) { logger.warn( 'Branch cannot automerge because it is behind base branch and rebaseWhen setting disallows rebasing - raising a PR instead', diff --git a/lib/workers/repository/update/branch/reuse.spec.ts b/lib/workers/repository/update/branch/reuse.spec.ts index 40856c5cd2..6ebe09a688 100644 --- a/lib/workers/repository/update/branch/reuse.spec.ts +++ b/lib/workers/repository/update/branch/reuse.spec.ts @@ -10,6 +10,7 @@ describe('workers/repository/update/branch/reuse', () => { sourceBranch: 'master', state: 'open', title: 'any', + labels: ['keep-updated'], }; let config: BranchConfig; @@ -189,5 +190,37 @@ describe('workers/repository/update/branch/reuse', () => { const res = await shouldReuseExistingBranch(config); expect(res.reuseExistingBranch).toBeTrue(); }); + + it('returns false if rebaseWhen=never, keepUpdatedLabel and stale', async () => { + config.rebaseWhen = 'never'; + config.keepUpdatedLabel = 'keep-updated'; + platform.getBranchPr.mockResolvedValueOnce(pr); + scm.branchExists.mockResolvedValueOnce(true); + scm.isBranchBehindBase.mockResolvedValueOnce(true); + const res = await shouldReuseExistingBranch(config); + expect(res.reuseExistingBranch).toBeFalse(); + }); + + it('returns false if rebaseWhen=conflicted, keepUpdatedLabel and modified', async () => { + config.rebaseWhen = 'never'; + config.keepUpdatedLabel = 'keep-updated'; + platform.getBranchPr.mockResolvedValue(pr); + scm.branchExists.mockResolvedValueOnce(true); + scm.isBranchConflicted.mockResolvedValueOnce(true); + scm.isBranchModified.mockResolvedValueOnce(false); + const res = await shouldReuseExistingBranch(config); + expect(res.reuseExistingBranch).toBeFalse(); + expect(res.isModified).toBeUndefined(); + }); + + it('returns true if rebaseWhen=never, miss-match keepUpdatedLabel and stale', async () => { + config.rebaseWhen = 'never'; + config.keepUpdatedLabel = 'keep-not-updated'; + platform.getBranchPr.mockResolvedValueOnce(pr); + scm.branchExists.mockResolvedValueOnce(true); + scm.isBranchBehindBase.mockResolvedValueOnce(true); + const res = await shouldReuseExistingBranch(config); + expect(res.reuseExistingBranch).toBeTrue(); + }); }); }); diff --git a/lib/workers/repository/update/branch/reuse.ts b/lib/workers/repository/update/branch/reuse.ts index 0de0afe1e2..90c79ae5d1 100644 --- a/lib/workers/repository/update/branch/reuse.ts +++ b/lib/workers/repository/update/branch/reuse.ts @@ -10,6 +10,28 @@ type ParentBranch = { isConflicted?: boolean; }; +async function shouldKeepUpdated( + config: BranchConfig, + baseBranch: string, + branchName: string, +): Promise<boolean> { + const keepUpdatedLabel = config.keepUpdatedLabel; + if (!keepUpdatedLabel) { + return false; + } + + const branchPr = await platform.getBranchPr( + config.branchName, + config.baseBranch, + ); + + if (branchPr?.labels?.includes(keepUpdatedLabel)) { + return true; + } + + return false; +} + export async function shouldReuseExistingBranch( config: BranchConfig, ): Promise<ParentBranch> { @@ -23,6 +45,7 @@ export async function shouldReuseExistingBranch( logger.debug(`Branch already exists`); if ( config.rebaseWhen === 'behind-base-branch' || + (await shouldKeepUpdated(config, baseBranch, branchName)) || (config.rebaseWhen === 'auto' && (config.automerge === true || (await platform.getBranchForceRebase?.(config.baseBranch)))) @@ -53,7 +76,10 @@ export async function shouldReuseExistingBranch( if ((await scm.isBranchModified(branchName)) === false) { logger.debug(`Branch is not mergeable and needs rebasing`); - if (config.rebaseWhen === 'never') { + if ( + config.rebaseWhen === 'never' && + !(await shouldKeepUpdated(config, baseBranch, branchName)) + ) { logger.debug('Rebasing disabled by config'); result.reuseExistingBranch = true; result.isModified = false; -- GitLab