Skip to content

Commit

Permalink
fix(github)!: change automerge priority order to prefer squash (#32016)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
pballandras authored Nov 1, 2024
1 parent 4757c06 commit fbd3680
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 28 deletions.
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
20 changes: 10 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) {
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 @@ -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;
}
Expand Down

0 comments on commit fbd3680

Please sign in to comment.