feat: add ignoreReviewers config option (#19776)

This commit is contained in:
Rostislav Simonik 2023-02-11 08:24:02 +01:00 committed by GitHub
parent e6e0ee9705
commit 05517e6d8a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
20 changed files with 146 additions and 41 deletions

View file

@ -1336,6 +1336,19 @@ For example, consider this config:
It would take the entire `"config:base"` preset - which has a lot of sub-presets - but ignore the `":prHourlyLimit2"` rule.
## ignoreReviewers
By default, Renovate does not add assignees or reviewers to PRs which are configured for automerge.
If tests have failed, Renovate then does add them, but only if the assignees and reviewers list is empty.
In the case that a user is automatically added as reviewer (such as Renovate Approve bot) and you want to ignore it for the purpose of this decision, add it to the `ignoreReviewers` list.
```json
{
"reviewers": ["foo"],
"ignoreReviewers": ["renovate-approve"]
}
```
## ignoreScripts
Applicable for npm and Composer only for now. Set this to `true` if running scripts causes problems.

View file

@ -137,6 +137,9 @@ If you have enabled branch protection which prevents Renovate from automerging d
When automerge is enabled on a PR, Renovate will _not_ add assignees or reviewers at PR creation time, in order to decrease notifications noise a little.
If tests subsequently _fail_, making automerge not possible, then Renovate will add the configured assignees and/or reviewers.
Note: Renovate won't add assignees and reviewers to a PR with failing checks if the PR already has assignees or reviewers present.
If there are accounts you wish to ignore (i.e. add assignees and reviewers regardless) then add them to `ignoreReviewers` to specify those which should be filtered out in such consideration.
## Frequent problems and how to resolve them
### Automerge not enabled correctly in config

View file

@ -1900,6 +1900,13 @@ const options: RenovateOptions[] = [
type: 'boolean',
default: false,
},
{
name: 'ignoreReviewers',
description:
'Reviewers to be ignored in PR reviewers presence (either username or email address depending on the platform).',
type: 'array',
subType: 'string',
},
{
name: 'reviewers',
description:

View file

@ -262,6 +262,7 @@ export interface AssigneesAndReviewersConfig {
assigneesFromCodeOwners?: boolean;
assignees?: string[];
assigneesSampleSize?: number;
ignoreReviewers?: string[];
reviewersFromCodeOwners?: boolean;
reviewers?: string[];
reviewersSampleSize?: number;

View file

@ -238,7 +238,6 @@ exports[`modules/platform/azure/index getPr(prNo) should return a pr in the righ
},
"createdAt": undefined,
"displayNumber": "Pull Request #1234",
"hasReviewers": false,
"labels": [
"renovate",
],

View file

@ -379,7 +379,6 @@ describe('modules/platform/azure/index', () => {
},
createdAt: undefined,
displayNumber: 'Pull Request #1',
hasReviewers: false,
labels: [],
number: 1,
pullRequestId: 1,

View file

@ -288,7 +288,6 @@ export async function getPr(pullRequestId: number): Promise<Pr | null> {
.filter((label) => label.active)
.map((label) => label.name)
.filter(is.string);
azurePr.hasReviewers = is.nonEmptyArray(azurePr.reviewers);
return azurePr;
}

View file

@ -32,7 +32,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getBranch
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -52,7 +51,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getPr() c
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -72,7 +70,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getPr() c
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -92,7 +89,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getPr() c
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -112,7 +108,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getPr() g
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -132,7 +127,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with no path getPr() g
},
"createdAt": undefined,
"displayNumber": "Pull Request #undefined",
"hasReviewers": false,
"number": undefined,
"reviewers": [],
"sourceBranch": undefined,
@ -248,7 +242,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getBranchPr(
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -268,7 +261,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getPr() canR
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -288,7 +280,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getPr() canR
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -308,7 +299,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getPr() canR
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -328,7 +318,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getPr() gets
},
"createdAt": 1547853840016,
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"number": 5,
"reviewers": [
"userName2",
@ -348,7 +337,6 @@ exports[`modules/platform/bitbucket-server/index endpoint with path getPr() gets
},
"createdAt": undefined,
"displayNumber": "Pull Request #undefined",
"hasReviewers": false,
"number": undefined,
"reviewers": [],
"sourceBranch": undefined,

View file

@ -1,4 +1,3 @@
import is from '@sindresorhus/is';
import delay from 'delay';
import JSON5 from 'json5';
import type { PartialDeep } from 'type-fest';
@ -265,7 +264,6 @@ export async function getPr(
...utils.prInfo(res.body),
reviewers: res.body.reviewers.map((r) => r.user.name),
};
pr.hasReviewers = is.nonEmptyArray(pr.reviewers);
// TODO #7154
pr.version = updatePrVersion(pr.number, pr.version!);

View file

@ -35,7 +35,6 @@ exports[`modules/platform/bitbucket/index getBranchPr() bitbucket finds PR for b
},
"createdAt": "2018-07-02T07:02:25.275030+00:00",
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"number": 5,
"sourceBranch": "branch",
"state": "open",
@ -70,7 +69,6 @@ exports[`modules/platform/bitbucket/index getPr() canRebase 1`] = `
},
"createdAt": "2018-07-02T07:02:25.275030+00:00",
"displayNumber": "Pull Request #3",
"hasReviewers": false,
"number": 3,
"sourceBranch": "branch",
"state": "open",
@ -86,7 +84,6 @@ exports[`modules/platform/bitbucket/index getPr() canRebase 2`] = `
},
"createdAt": "2018-07-02T07:02:25.275030+00:00",
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"number": 5,
"sourceBranch": "branch",
"state": "open",
@ -102,7 +99,6 @@ exports[`modules/platform/bitbucket/index getPr() canRebase 3`] = `
},
"createdAt": "2018-07-02T07:02:25.275030+00:00",
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"number": 5,
"sourceBranch": "branch",
"state": "open",
@ -118,7 +114,6 @@ exports[`modules/platform/bitbucket/index getPr() exists 1`] = `
},
"createdAt": "2018-07-02T07:02:25.275030+00:00",
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"number": 5,
"sourceBranch": "branch",
"state": "open",

View file

@ -985,6 +985,32 @@ describe('modules/platform/bitbucket/index', () => {
expect(await bitbucket.getPr(5)).toMatchSnapshot();
});
it('reviewers', async () => {
const reviewer = {
display_name: 'Jane Smith',
uuid: '{90b6646d-1724-4a64-9fd9-539515fe94e9}',
account_id: '456',
};
const scope = await initRepoMock();
scope.get('/2.0/repositories/some/repo/pullrequests/5').reply(200, {
...pr,
reviewers: [reviewer],
});
expect(await bitbucket.getPr(5)).toEqual({
bodyStruct: {
hash: '761b7ad8ad439b2855fcbb611331c646ef0870b0631247bba3f3025cb6df5a53',
},
createdAt: '2018-07-02T07:02:25.275030+00:00',
displayNumber: 'Pull Request #5',
number: 5,
reviewers: ['{90b6646d-1724-4a64-9fd9-539515fe94e9}'],
sourceBranch: 'branch',
state: 'open',
targetBranch: 'master',
title: 'title',
});
});
});
describe('massageMarkdown()', () => {

View file

@ -290,12 +290,16 @@ export async function getPr(prNo: number): Promise<Pr | null> {
return null;
}
const res: any = {
const res: Pr = {
displayNumber: `Pull Request #${pr.id}`,
...utils.prInfo(pr),
};
res.hasReviewers = is.nonEmptyArray(pr.reviewers);
if (is.nonEmptyArray(pr.reviewers)) {
res.reviewers = pr.reviewers
.map(({ uuid }) => uuid)
.filter(is.nonEmptyString);
}
return res;
}

View file

@ -38,7 +38,9 @@ export function coerceRestPr(pr: GhRestPr): GhPr {
}
if (pr.requested_reviewers) {
result.hasReviewers = true;
result.reviewers = pr.requested_reviewers
.map(({ login }) => login)
.filter(is.nonEmptyString);
}
if (pr.created_at) {

View file

@ -2580,7 +2580,6 @@ describe('modules/platform/github/index', () => {
},
displayNumber: 'Pull Request #1234',
hasAssignees: true,
hasReviewers: true,
number: 1234,
sourceBranch: 'some/branch',
state: 'open',

View file

@ -75,10 +75,10 @@ exports[`modules/platform/gitlab/index getBranchPr(branchName) should return the
},
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"labels": undefined,
"number": 91,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "open",
@ -94,11 +94,11 @@ exports[`modules/platform/gitlab/index getBranchPr(branchName) should strip depr
},
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"isDraft": true,
"labels": undefined,
"number": 91,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "open",
@ -114,11 +114,11 @@ exports[`modules/platform/gitlab/index getBranchPr(branchName) should strip draf
},
"displayNumber": "Merge Request #91",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"isDraft": true,
"labels": undefined,
"number": 91,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "open",
@ -134,11 +134,11 @@ exports[`modules/platform/gitlab/index getPr(prNo) removes deprecated draft pref
},
"displayNumber": "Merge Request #12345",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"isDraft": true,
"labels": undefined,
"number": 12345,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "merged",
@ -154,11 +154,11 @@ exports[`modules/platform/gitlab/index getPr(prNo) removes draft prefix from ret
},
"displayNumber": "Merge Request #12345",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"isDraft": true,
"labels": undefined,
"number": 12345,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "merged",
@ -174,10 +174,10 @@ exports[`modules/platform/gitlab/index getPr(prNo) returns the PR 1`] = `
},
"displayNumber": "Merge Request #12345",
"hasAssignees": false,
"hasReviewers": false,
"headPipelineStatus": undefined,
"labels": undefined,
"number": 12345,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "merged",
@ -193,10 +193,10 @@ exports[`modules/platform/gitlab/index getPr(prNo) returns the PR with nonexisti
},
"displayNumber": "Merge Request #12345",
"hasAssignees": true,
"hasReviewers": false,
"headPipelineStatus": undefined,
"labels": undefined,
"number": 12345,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "open",
@ -212,10 +212,10 @@ exports[`modules/platform/gitlab/index getPr(prNo) returns the mergeable PR 1`]
},
"displayNumber": "Merge Request #12345",
"hasAssignees": true,
"hasReviewers": false,
"headPipelineStatus": undefined,
"labels": undefined,
"number": 12345,
"reviewers": undefined,
"sha": undefined,
"sourceBranch": "some-branch",
"state": "open",

View file

@ -1919,6 +1919,47 @@ describe('modules/platform/gitlab/index', () => {
expect(pr).toMatchSnapshot();
expect(pr?.hasAssignees).toBeTrue();
});
it('returns the PR with reviewers', async () => {
httpMock
.scope(gitlabApiHost)
.get(
'/api/v4/projects/undefined/merge_requests/12345?include_diverged_commits_count=1'
)
.reply(200, {
id: 1,
iid: 12345,
title: 'do something',
description: 'a merge request',
state: 'merged',
merge_status: 'cannot_be_merged',
diverged_commits_count: 5,
source_branch: 'some-branch',
target_branch: 'master',
assignees: [],
reviewers: [
{ id: 1, username: 'foo' },
{ id: 2, username: 'bar' },
],
});
const pr = await gitlab.getPr(12345);
expect(pr).toEqual({
bodyStruct: {
hash: '23f41dbec0785a6c77457dd6ebf99ae5970c5fffc9f7a8ad7f66c1b8eeba5b90',
},
displayNumber: 'Merge Request #12345',
hasAssignees: false,
headPipelineStatus: undefined,
labels: undefined,
number: 12345,
reviewers: ['foo', 'bar'],
sha: undefined,
sourceBranch: 'some-branch',
state: 'merged',
targetBranch: 'master',
title: 'do something',
});
});
});
describe('updatePr(prNo, title, body)', () => {

View file

@ -628,7 +628,7 @@ export async function getPr(iid: number): Promise<GitlabPr> {
state: mr.state === 'opened' ? 'open' : mr.state,
headPipelineStatus: mr.head_pipeline?.status,
hasAssignees: !!(mr.assignee?.id ?? mr.assignees?.[0]?.id),
hasReviewers: !!mr.reviewers?.length,
reviewers: mr.reviewers?.map(({ username }) => username),
title: mr.title,
labels: mr.labels,
sha: mr.sha,

View file

@ -67,7 +67,6 @@ export interface Pr {
closedAt?: string;
displayNumber?: string;
hasAssignees?: boolean;
hasReviewers?: boolean;
labels?: string[];
number: number;
reviewers?: string[];

View file

@ -361,7 +361,6 @@ describe('workers/repository/update/pr/index', () => {
const changedPr: Pr = {
...pr,
hasAssignees: false,
hasReviewers: false,
};
platform.getBranchPr.mockResolvedValueOnce(changedPr);
checks.resolveBranchStatus.mockResolvedValueOnce('red');
@ -377,6 +376,27 @@ describe('workers/repository/update/pr/index', () => {
expect(participants.addParticipants).toHaveBeenCalled();
});
it('adds reviewers for PR automerge with red status and existing ignorable reviewers that can be ignored', async () => {
const changedPr: Pr = {
...pr,
hasAssignees: false,
reviewers: ['renovate-approve'],
};
platform.getBranchPr.mockResolvedValueOnce(changedPr);
checks.resolveBranchStatus.mockResolvedValueOnce('red');
const res = await ensurePr({
...config,
automerge: true,
automergeType: 'pr',
assignAutomerge: false,
ignoreReviewers: ['renovate-approve'],
});
expect(res).toEqual({ type: 'with-pr', pr: changedPr });
expect(participants.addParticipants).toHaveBeenCalled();
});
it('skips branch automerge and forces PR creation due to artifact errors', async () => {
platform.createPr.mockResolvedValueOnce(pr);
@ -479,7 +499,6 @@ describe('workers/repository/update/pr/index', () => {
const changedPr: Pr = {
...pr,
hasAssignees: false,
hasReviewers: false,
};
platform.getBranchPr.mockResolvedValueOnce(changedPr);
checks.resolveBranchStatus.mockResolvedValueOnce('red');
@ -504,7 +523,6 @@ describe('workers/repository/update/pr/index', () => {
const changedPr: Pr = {
...pr,
hasAssignees: false,
hasReviewers: false,
};
platform.getBranchPr.mockResolvedValueOnce(changedPr);
checks.resolveBranchStatus.mockResolvedValueOnce('red');
@ -533,7 +551,6 @@ describe('workers/repository/update/pr/index', () => {
const changedPr: Pr = {
...pr,
hasAssignees: false,
hasReviewers: false,
};
platform.getBranchPr.mockResolvedValueOnce(changedPr);
checks.resolveBranchStatus.mockResolvedValueOnce('red');

View file

@ -74,6 +74,20 @@ export function updatePrDebugData(
};
}
function hasNotIgnoredReviewers(pr: Pr, config: BranchConfig): boolean {
if (
is.nonEmptyArray(config.ignoreReviewers) &&
is.nonEmptyArray(pr.reviewers)
) {
const ignoreReviewers = new Set(config.ignoreReviewers);
return (
pr.reviewers.filter((reviewer) => !ignoreReviewers.has(reviewer)).length >
0
);
}
return pr.reviewers ? pr.reviewers.length > 0 : false;
}
// Ensures that PR exists with matching title/body
export async function ensurePr(
prConfig: BranchConfig
@ -273,9 +287,10 @@ export async function ensurePr(
try {
if (existingPr) {
logger.debug('Processing existing PR');
if (
!existingPr.hasAssignees &&
!existingPr.hasReviewers &&
!hasNotIgnoredReviewers(existingPr, config) &&
config.automerge &&
!config.assignAutomerge &&
(await getBranchStatus()) === 'red'