refactor: drop pr.isStale (#6826)

This commit is contained in:
Rhys Arkins 2020-07-23 10:15:51 +02:00 committed by GitHub
parent d6bfaa665f
commit eaffc19253
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 15 additions and 186 deletions

View file

@ -1423,7 +1423,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -1862,7 +1861,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -1884,7 +1882,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -1906,7 +1903,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -2029,7 +2025,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -2101,7 +2096,6 @@ Object {
"createdAt": undefined,
"displayNumber": "Pull Request #undefined",
"hasReviewers": false,
"isStale": false,
"number": undefined,
"reviewers": Array [],
"state": "merged",
@ -5047,7 +5041,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -5486,7 +5479,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -5508,7 +5500,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -5530,7 +5521,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -5653,7 +5643,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": true,
"isConflicted": false,
"isStale": false,
"number": 5,
"reviewers": Array [
"userName2",
@ -5725,7 +5714,6 @@ Object {
"createdAt": undefined,
"displayNumber": "Pull Request #undefined",
"hasReviewers": false,
"isStale": false,
"number": undefined,
"reviewers": Array [],
"state": "merged",

View file

@ -280,10 +280,6 @@ export async function getPr(
pr.canMerge = !!mergeRes.body.canMerge;
}
if (await git.branchExists(pr.branchName)) {
pr.isStale = await git.isBranchStale(pr.branchName);
}
return pr;
}

View file

@ -509,7 +509,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"isConflicted": false,
"isStale": false,
"number": 5,
"state": "open",
"targetBranch": "master",
@ -899,7 +898,6 @@ Object {
"displayNumber": "Pull Request #3",
"hasReviewers": false,
"isConflicted": false,
"isStale": false,
"number": 3,
"state": "open",
"targetBranch": "master",
@ -916,7 +914,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"isConflicted": false,
"isStale": false,
"number": 5,
"state": "open",
"targetBranch": "master",
@ -933,7 +930,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"isConflicted": false,
"isStale": false,
"number": 5,
"state": "open",
"targetBranch": "master",
@ -1029,7 +1025,6 @@ Object {
"displayNumber": "Pull Request #5",
"hasReviewers": false,
"isConflicted": false,
"isStale": false,
"number": 5,
"state": "open",
"targetBranch": "master",

View file

@ -290,10 +290,6 @@ export async function getPr(prNo: number): Promise<Pr | null> {
// TODO: Is that correct? Should we check getBranchStatus like gitlab?
res.canMerge = !res.isConflicted;
}
if (await git.branchExists(pr.source.branch.name)) {
res.isStale = await git.isBranchStale(pr.source.branch.name);
}
res.hasReviewers = is.nonEmptyArray(pr.reviewers);
return res;

View file

@ -46,7 +46,6 @@ export interface Pr {
hasAssignees?: boolean;
hasReviewers?: boolean;
isConflicted?: boolean;
isStale?: boolean;
labels?: string[];
number?: number;
reviewers?: string[];

View file

@ -9,7 +9,6 @@ Object {
"displayNumber": "Pull Request #42",
"hasAssignees": false,
"isConflicted": false,
"isStale": undefined,
"number": 42,
"sha": "0d9c7726c3d628b7e28af234595cfd20febdbf8e",
"sourceRepo": "some/repo",
@ -28,7 +27,6 @@ Object {
"displayNumber": "Pull Request #42",
"hasAssignees": false,
"isConflicted": false,
"isStale": undefined,
"number": 42,
"sha": "0d9c7726c3d628b7e28af234595cfd20febdbf8e",
"sourceRepo": "some/repo",
@ -47,7 +45,6 @@ Object {
"displayNumber": "Pull Request #1",
"hasAssignees": false,
"isConflicted": false,
"isStale": false,
"number": 1,
"sha": "some-head-sha",
"sourceRepo": "some/repo",
@ -66,7 +63,6 @@ Object {
"displayNumber": "Pull Request #1",
"hasAssignees": false,
"isConflicted": false,
"isStale": false,
"number": 1,
"sha": "some-head-sha",
"sourceRepo": "some/repo",
@ -86,7 +82,6 @@ Array [
"displayNumber": "Pull Request #1",
"hasAssignees": false,
"isConflicted": false,
"isStale": undefined,
"number": 1,
"sha": "some-head-sha",
"sourceRepo": "some/repo",
@ -102,7 +97,6 @@ Array [
"displayNumber": "Pull Request #2",
"hasAssignees": false,
"isConflicted": false,
"isStale": undefined,
"number": 2,
"sha": "other-head-sha",
"sourceRepo": "some/repo",

View file

@ -103,7 +103,6 @@ function toRenovatePR(data: helper.PR): Pr | null {
createdAt: data.created_at,
canMerge: data.mergeable,
isConflicted: !data.mergeable,
isStale: undefined,
hasAssignees: !!(data.assignee?.login || is.nonEmptyArray(data.assignees)),
};
}
@ -464,13 +463,6 @@ const platform: Platform = {
return null;
}
// Enrich pull request with additional information which is more expensive to fetch
if (pr.state !== 'closed') {
if (pr.isStale === undefined) {
pr.isStale = await git.isBranchStale(pr.branchName);
}
}
return pr;
},

View file

@ -1866,7 +1866,6 @@ Object {
"full_name": "some/repo",
},
},
"isStale": true,
"number": 91,
"sha": undefined,
"state": "open",
@ -2057,17 +2056,6 @@ Array [
"method": "GET",
"url": "https://api.github.com/repos/some/repo/pulls/91",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/some/repo/git/refs/heads/master",
},
]
`;
@ -2089,7 +2077,6 @@ Object {
"full_name": "other/repo",
},
},
"isStale": true,
"number": 90,
"sha": undefined,
"state": "open",
@ -2327,17 +2314,6 @@ Array [
"method": "GET",
"url": "https://api.github.com/repos/some/repo/pulls/90",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/forked/repo/git/refs/heads/master",
},
]
`;
@ -3007,17 +2983,6 @@ Array [
"method": "POST",
"url": "https://api.github.com/graphql",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/some/repo/git/refs/heads/master",
},
Object {
"graphql": Object {
"query": Object {
@ -3073,7 +3038,6 @@ Object {
"hasAssignees": false,
"hasReviewers": false,
"isConflicted": true,
"isStale": true,
"number": 2500,
"state": "open",
"targetBranch": "master",
@ -3200,17 +3164,6 @@ Array [
"method": "POST",
"url": "https://api.github.com/graphql",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/some/repo/git/refs/heads/master",
},
]
`;
@ -3594,17 +3547,6 @@ Array [
"method": "GET",
"url": "https://api.github.com/repos/some/repo/pulls/1234",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/some/repo/git/refs/heads/master",
},
]
`;
@ -3617,7 +3559,6 @@ Object {
"canMerge": true,
"commits": 1,
"displayNumber": "Pull Request #1",
"isStale": true,
"mergeable": true,
"number": 1,
"sha": undefined,
@ -3798,17 +3739,6 @@ Array [
"method": "GET",
"url": "https://api.github.com/repos/some/repo/pulls/1234",
},
Object {
"headers": Object {
"accept": "application/vnd.github.v3+json",
"accept-encoding": "gzip, deflate",
"authorization": "token abc123",
"host": "api.github.com",
"user-agent": "https://github.com/renovatebot/renovate",
},
"method": "GET",
"url": "https://api.github.com/repos/some/repo/git/refs/heads/master",
},
]
`;

View file

@ -491,9 +491,7 @@ describe('platform/github', () => {
},
head: { ref: 'somebranch', repo: { full_name: 'some/repo' } },
state: 'open',
})
.get('/repos/some/repo/git/refs/heads/master')
.reply(200, { object: { sha: '12345' } });
});
await github.initRepo({
repository: 'some/repo',
@ -534,8 +532,6 @@ describe('platform/github', () => {
head: { ref: 'somebranch', repo: { full_name: 'other/repo' } },
state: 'open',
})
.get('/repos/forked/repo/git/refs/heads/master')
.reply(200, { object: { sha: '12345' } })
.patch('/repos/forked/repo/git/refs/heads/master')
.reply(200);
await github.initRepo({
@ -1643,15 +1639,7 @@ describe('platform/github', () => {
it('should return PR from graphql result', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope
.post('/graphql')
.reply(200, graphqlOpenPullRequests)
.get('/repos/some/repo/git/refs/heads/master')
.reply(200, {
object: {
sha: '1234123412341234123412341234123412341234',
},
});
scope.post('/graphql').reply(200, graphqlOpenPullRequests);
global.gitAuthor = {
name: 'Renovate Bot',
email: 'renovate@whitesourcesoftware.com',
@ -1672,12 +1660,7 @@ describe('platform/github', () => {
.reply(200, graphqlOpenPullRequests)
.post('/graphql')
.reply(200, graphqlClosedPullRequests)
.get('/repos/some/repo/git/refs/heads/master')
.reply(200, {
object: {
sha: '1234123412341234123412341234123412341234',
},
});
.get('/repos/some/repo/git/refs/heads/master');
await github.initRepo({
repository: 'some/repo',
} as any);
@ -1735,12 +1718,6 @@ describe('platform/github', () => {
base: { sha: '1234' },
commits: 1,
})
.get('/repos/some/repo/git/refs/heads/master')
.reply(200, {
object: {
sha: '1234',
},
})
.post('/graphql')
.twice()
.reply(404);
@ -1764,12 +1741,6 @@ describe('platform/github', () => {
commits: 1,
mergeable: true,
})
.get('/repos/some/repo/git/refs/heads/master')
.reply(200, {
object: {
sha: '1234',
},
})
.post('/graphql')
.twice()
.reply(404);

View file

@ -681,18 +681,6 @@ async function getOpenPrs(): Promise<PrList> {
} else {
pr.isConflicted = false;
}
pr.isStale = false;
if (pr.mergeStateStatus === 'BEHIND') {
pr.isStale = true;
} else {
const baseCommitSHA = await getBaseCommitSHA();
if (
pr.commits?.nodes[0]?.commit?.parents?.edges?.[0]?.node?.oid !==
baseCommitSHA
) {
pr.isStale = true;
}
}
if (pr.labels) {
pr.labels = pr.labels.nodes.map(
(label: { name: string }) => label.name
@ -761,10 +749,6 @@ export async function getPr(prNo: number): Promise<Pr | null> {
logger.debug({ prNo }, 'PR state is dirty so unmergeable');
pr.isConflicted = true;
}
const baseCommitSHA = await getBaseCommitSHA();
if (!pr.base || pr.base.sha !== baseCommitSHA) {
pr.isStale = true;
}
}
return pr;
}

View file

@ -817,7 +817,6 @@ Object {
"hasAssignees": false,
"hasReviewers": false,
"iid": 91,
"isStale": false,
"number": 91,
"source_branch": "some-branch",
"state": "open",
@ -1147,7 +1146,6 @@ Object {
"id": 1,
"iid": 12345,
"isConflicted": true,
"isStale": true,
"merge_status": "cannot_be_merged",
"number": 12345,
"source_branch": "some-branch",
@ -1186,7 +1184,6 @@ Object {
"id": 1,
"iid": 12345,
"isConflicted": true,
"isStale": true,
"merge_status": "cannot_be_merged",
"number": 12345,
"source_branch": "some-branch",
@ -1224,7 +1221,6 @@ Object {
"hasReviewers": false,
"id": 1,
"iid": 12345,
"isStale": true,
"number": 12345,
"source_branch": "some-branch",
"state": "open",

View file

@ -447,7 +447,6 @@ export async function getPr(iid: number): Promise<Pr> {
pr.number = pr.iid;
pr.displayNumber = `Merge Request #${pr.iid}`;
pr.body = pr.description;
pr.isStale = pr.diverged_commits_count > 0;
pr.state = pr.state === 'opened' ? PR_STATE_OPEN : pr.state;
pr.hasAssignees = !!(pr.assignee?.id || pr.assignees?.[0]?.id);
delete pr.assignee;

View file

@ -1,11 +1,4 @@
import { mock } from 'jest-mock-extended';
import {
RenovateConfig,
defaultConfig,
git,
platform,
} from '../../../../../test/util';
import { Pr } from '../../../../platform';
import { RenovateConfig, defaultConfig, git } from '../../../../../test/util';
import { rebaseOnboardingBranch } from './rebase';
jest.mock('../../../../util/git');
@ -20,9 +13,6 @@ describe('workers/repository/onboarding/branch/rebase', () => {
};
});
it('does not rebase modified branch', async () => {
platform.getBranchPr.mockResolvedValueOnce({
...mock<Pr>(),
});
git.isBranchModified.mockResolvedValueOnce(true);
await rebaseOnboardingBranch(config);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
@ -33,18 +23,11 @@ describe('workers/repository/onboarding/branch/rebase', () => {
git.getFile
.mockResolvedValueOnce(contents) // package.json
.mockResolvedValueOnce(contents); // renovate.json
platform.getBranchPr.mockResolvedValueOnce({
...mock<Pr>(),
isStale: false,
});
await rebaseOnboardingBranch(config);
expect(git.commitFiles).toHaveBeenCalledTimes(0);
});
it('rebases onboarding branch', async () => {
platform.getBranchPr.mockResolvedValueOnce({
...mock<Pr>(),
isStale: true,
});
git.isBranchStale.mockResolvedValueOnce(true);
await rebaseOnboardingBranch(config);
expect(git.commitFiles).toHaveBeenCalledTimes(1);
});

View file

@ -1,8 +1,12 @@
import { RenovateConfig } from '../../../../config';
import { configFileNames } from '../../../../config/app-strings';
import { logger } from '../../../../logger';
import { platform } from '../../../../platform';
import { commitFiles, getFile, isBranchModified } from '../../../../util/git';
import {
commitFiles,
getFile,
isBranchModified,
isBranchStale,
} from '../../../../util/git';
import { getOnboardingConfig } from './config';
const defaultConfigFile = configFileNames[0];
@ -27,7 +31,6 @@ export async function rebaseOnboardingBranch(
config: RenovateConfig
): Promise<string | null> {
logger.debug('Checking if onboarding branch needs rebasing');
const pr = await platform.getBranchPr(config.onboardingBranch);
if (await isBranchModified(config.onboardingBranch)) {
logger.debug('Onboarding branch has been edited and cannot be rebased');
return null;
@ -37,7 +40,10 @@ export async function rebaseOnboardingBranch(
config.onboardingBranch
);
const contents = getOnboardingConfig(config);
if (contents === existingContents && !pr.isStale) {
if (
contents === existingContents &&
!(await isBranchStale(config.onboardingBranch))
) {
logger.debug('Onboarding branch is up to date');
return null;
}