From b0bc3e8ffc24f284d1f28ba5a6984cdec39244c4 Mon Sep 17 00:00:00 2001 From: Philippe Ballandras <pballandras@coveo.com> Date: Fri, 1 Nov 2024 07:23:55 -0400 Subject: [PATCH] fix(github)!: change automerge priority order to prefer squash (#32016) Changes the priority order so that squash merges are preferred over rebase merge and merge commits. Doing so allows GitHub to sign the resulting commit on the base branch. BREAKING CHANGE: Renovate will now prefer squash merges over others in GitHub, if they are allowed. --- lib/modules/platform/github/index.spec.ts | 36 +++++++++++------------ lib/modules/platform/github/index.ts | 20 ++++++------- 2 files changed, 28 insertions(+), 28 deletions(-) diff --git a/lib/modules/platform/github/index.spec.ts b/lib/modules/platform/github/index.spec.ts index e7e4ea1b89..374c60079c 100644 --- a/lib/modules/platform/github/index.spec.ts +++ b/lib/modules/platform/github/index.spec.ts @@ -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' }); @@ -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`) @@ -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: { @@ -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`) @@ -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', @@ -2882,7 +2882,7 @@ describe('modules/platform/github/index', () => { }, variables: { pullRequestId: 'abcd', - mergeMethod: 'REBASE', + mergeMethod: 'SQUASH', }, }, }; @@ -3510,7 +3510,7 @@ describe('modules/platform/github/index', () => { }, variables: { pullRequestId: 'abcd', - mergeMethod: 'REBASE', + mergeMethod: 'SQUASH', }, }, }; @@ -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); @@ -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, @@ -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 = { @@ -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 = { diff --git a/lib/modules/platform/github/index.ts b/lib/modules/platform/github/index.ts index 08678c06d3..082932f347 100644 --- a/lib/modules/platform/github/index.ts +++ b/lib/modules/platform/github/index.ts @@ -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) { 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'); @@ -1910,25 +1910,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 + 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; } -- GitLab