Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(github)!: change automerge strategy priority order to allow platform signed commits #32016

Merged
36 changes: 18 additions & 18 deletions lib/modules/platform/github/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,7 @@ describe('modules/platform/github/index', () => {
}

describe('initRepo', () => {
it('should rebase', async () => {
it('should squash', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
const config = await github.initRepo({ repository: 'some/repo' });
Expand Down Expand Up @@ -675,7 +675,7 @@ describe('modules/platform/github/index', () => {
expect(config).toMatchSnapshot();
});

it('should squash', async () => {
it('should merge', async () => {
httpMock
.scope(githubApiHost)
.post(`/graphql`)
Expand All @@ -687,8 +687,8 @@ describe('modules/platform/github/index', () => {
nameWithOwner: 'some/repo',
hasIssuesEnabled: true,
mergeCommitAllowed: true,
rebaseMergeAllowed: false,
squashMergeAllowed: true,
rebaseMergeAllowed: true,
squashMergeAllowed: false,
defaultBranchRef: {
name: 'master',
target: {
Expand All @@ -704,7 +704,7 @@ describe('modules/platform/github/index', () => {
expect(config).toMatchSnapshot();
});

it('should merge', async () => {
it('should rebase', async () => {
httpMock
.scope(githubApiHost)
.post(`/graphql`)
Expand All @@ -715,8 +715,8 @@ describe('modules/platform/github/index', () => {
isArchived: false,
nameWithOwner: 'some/repo',
hasIssuesEnabled: true,
mergeCommitAllowed: true,
rebaseMergeAllowed: false,
mergeCommitAllowed: false,
rebaseMergeAllowed: true,
squashMergeAllowed: false,
defaultBranchRef: {
name: 'master',
Expand Down Expand Up @@ -2882,7 +2882,7 @@ describe('modules/platform/github/index', () => {
},
variables: {
pullRequestId: 'abcd',
mergeMethod: 'REBASE',
mergeMethod: 'SQUASH',
},
},
};
Expand Down Expand Up @@ -3510,7 +3510,7 @@ describe('modules/platform/github/index', () => {
},
variables: {
pullRequestId: 'abcd',
mergeMethod: 'REBASE',
mergeMethod: 'SQUASH',
},
},
};
Expand Down Expand Up @@ -3683,7 +3683,7 @@ describe('modules/platform/github/index', () => {
});

describe('mergePr(prNo) - autodetection', () => {
it('should try rebase first', async () => {
it('should try squash first', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope.put('/repos/some/repo/pulls/1235/merge').reply(200);
Expand All @@ -3702,12 +3702,12 @@ describe('modules/platform/github/index', () => {
).toBeTrue();
});

it('should try squash after rebase', async () => {
it('should try merge after squash', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope
.put('/repos/some/repo/pulls/1236/merge')
.reply(400, 'no rebasing allowed');
.reply(400, 'no squashing allowed');
await github.initRepo({ repository: 'some/repo' });
const pr = {
number: 1236,
Expand All @@ -3723,15 +3723,15 @@ describe('modules/platform/github/index', () => {
).toBeFalse();
});

it('should try merge after squash', async () => {
it('should try rebase after merge', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope
.put('/repos/some/repo/pulls/1237/merge')
.reply(405, 'no rebasing allowed')
.put('/repos/some/repo/pulls/1237/merge')
.reply(405, 'no squashing allowed')
.put('/repos/some/repo/pulls/1237/merge')
.reply(405, 'no merging allowed')
.put('/repos/some/repo/pulls/1237/merge')
.reply(200);
await github.initRepo({ repository: 'some/repo' });
const pr = {
Expand All @@ -3753,12 +3753,12 @@ describe('modules/platform/github/index', () => {
initRepoMock(scope, 'some/repo');
scope
.put('/repos/some/repo/pulls/1237/merge')
.reply(405, 'no rebasing allowed')
.put('/repos/some/repo/pulls/1237/merge')
.replyWithError('no squashing allowed')
.reply(405, 'no squashing allowed')
.put('/repos/some/repo/pulls/1237/merge')
.replyWithError('no merging allowed')
.put('/repos/some/repo/pulls/1237/merge')
.replyWithError('no rebasing allowed')
.put('/repos/some/repo/pulls/1237/merge')
.replyWithError('never gonna give you up');
await github.initRepo({ repository: 'some/repo' });
const pr = {
Expand Down
21 changes: 11 additions & 10 deletions lib/modules/platform/github/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -553,12 +553,12 @@ export async function initRepo({
// Base branch may be configured but defaultBranch is always fixed
logger.debug(`${repository} default branch = ${config.defaultBranch}`);
// GitHub allows administrators to block certain types of merge, so we need to check it
if (repo.rebaseMergeAllowed) {
config.mergeMethod = 'rebase';
} else if (repo.squashMergeAllowed) {
if (repo.squashMergeAllowed) {
pballandras marked this conversation as resolved.
Show resolved Hide resolved
config.mergeMethod = 'squash';
} else if (repo.mergeCommitAllowed) {
config.mergeMethod = 'merge';
} else if (repo.rebaseMergeAllowed) {
config.mergeMethod = 'rebase';
} else {
// This happens if we don't have Administrator read access, it is not a critical error
logger.debug('Could not find allowed merge methods for repo');
Expand Down Expand Up @@ -1851,6 +1851,7 @@ export async function reattemptPlatformAutomerge({
export async function mergePr({
branchName,
id: prNo,
strategy,
pballandras marked this conversation as resolved.
Show resolved Hide resolved
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${prNo}, ${branchName})`);
const url = `repos/${
Expand Down Expand Up @@ -1910,25 +1911,25 @@ export async function mergePr({
}
}
if (!automerged) {
// We need to guess the merge method and try squash -> rebase -> merge
options.body.merge_method = 'rebase';
// We need to guess the merge method and try squash -> merge -> rebase
pballandras marked this conversation as resolved.
Show resolved Hide resolved
options.body.merge_method = 'squash';
try {
logger.debug({ options, url }, `mergePr`);
automergeResult = await githubApi.putJson(url, options);
} catch (err1) {
logger.debug({ err: err1 }, `Failed to rebase merge PR`);
logger.debug({ err: err1 }, `Failed to squash merge PR`);
try {
options.body.merge_method = 'squash';
options.body.merge_method = 'merge';
logger.debug({ options, url }, `mergePr`);
automergeResult = await githubApi.putJson(url, options);
} catch (err2) {
logger.debug({ err: err2 }, `Failed to merge squash PR`);
logger.debug({ err: err2 }, `Failed to merge commit PR`);
try {
options.body.merge_method = 'merge';
options.body.merge_method = 'rebase';
logger.debug({ options, url }, `mergePr`);
automergeResult = await githubApi.putJson(url, options);
} catch (err3) {
logger.debug({ err: err3 }, `Failed to merge commit PR`);
logger.debug({ err: err3 }, `Failed to rebase merge PR`);
logger.info({ pr: prNo }, 'All merge attempts failed');
return false;
pballandras marked this conversation as resolved.
Show resolved Hide resolved
}
Expand Down