Skip to content

Commit

Permalink
feat: Add new keepUpdatedLabel config setting (#27542)
Browse files Browse the repository at this point in the history
Co-authored-by: Rhys Arkins <rhys@arkins.net>
  • Loading branch information
mheffner and rarkins committed Mar 1, 2024
1 parent 99467af commit 62d678d
Show file tree
Hide file tree
Showing 8 changed files with 165 additions and 2 deletions.
9 changes: 9 additions & 0 deletions docs/usage/configuration-options.md
Expand Up @@ -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.
Expand Down
1 change: 1 addition & 0 deletions docs/usage/updating-rebasing.md
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions lib/config/options/index.ts
Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions lib/config/types.ts
Expand Up @@ -58,6 +58,7 @@ export interface RenovateSharedConfig {
ignorePaths?: string[];
ignoreTests?: boolean;
internalChecksAsSuccess?: boolean;
keepUpdatedLabel?: string;
labels?: string[];
addLabels?: string[];
dependencyDashboardApproval?: boolean;
Expand Down
83 changes: 83 additions & 0 deletions lib/workers/repository/update/branch/index.spec.ts
Expand Up @@ -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>(),
Expand Down Expand Up @@ -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,
Expand Down
5 changes: 4 additions & 1 deletion lib/workers/repository/update/branch/index.ts
Expand Up @@ -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
Expand Down Expand Up @@ -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');
Expand Down Expand Up @@ -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',
Expand Down
33 changes: 33 additions & 0 deletions lib/workers/repository/update/branch/reuse.spec.ts
Expand Up @@ -10,6 +10,7 @@ describe('workers/repository/update/branch/reuse', () => {
sourceBranch: 'master',
state: 'open',
title: 'any',
labels: ['keep-updated'],
};
let config: BranchConfig;

Expand Down Expand Up @@ -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();
});
});
});
28 changes: 27 additions & 1 deletion lib/workers/repository/update/branch/reuse.ts
Expand Up @@ -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> {
Expand All @@ -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))))
Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 62d678d

Please sign in to comment.