Skip to content

Commit

Permalink
feat(github): support automergeStrategy (#34537)
Browse files Browse the repository at this point in the history
  • Loading branch information
RahulGautamSingh authored Feb 28, 2025
1 parent 5897f30 commit 72a5af8
Show file tree
Hide file tree
Showing 5 changed files with 74 additions and 9 deletions.
2 changes: 1 addition & 1 deletion lib/config/options/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2010,7 +2010,7 @@ const options: RenovateOptions[] = [
type: 'string',
allowedValues: ['auto', 'fast-forward', 'merge-commit', 'rebase', 'squash'],
default: 'auto',
supportedPlatforms: ['azure', 'bitbucket', 'gitea'],
supportedPlatforms: ['azure', 'bitbucket', 'gitea', 'github'],
},
{
name: 'automergeComment',
Expand Down
21 changes: 21 additions & 0 deletions lib/modules/platform/github/common.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import is from '@sindresorhus/is';
import { logger } from '../../../logger';
import { GithubHttp } from '../../../util/http/github';
import { getPrBodyStruct } from '../pr-body';
import type { MergePRConfig } from '../types';
import type { GhPr, GhRestPr } from './types';

export const githubApi = new GithubHttp();
Expand Down Expand Up @@ -57,3 +59,22 @@ export function coerceRestPr(pr: GhRestPr): GhPr {

return result;
}

export function mapMergeStartegy(
strategy: MergePRConfig['strategy'],
): string | undefined {
switch (strategy) {
case 'auto':
return undefined;
case 'fast-forward': {
logger.warn(
'Fast-forward merge strategy is not supported by Github. Falling back to merge strategy set for the repository.',
);
return undefined;
}
case 'merge-commit':
return 'merge';
default:
return strategy;
}
}
44 changes: 44 additions & 0 deletions lib/modules/platform/github/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3640,6 +3640,7 @@ describe('modules/platform/github/index', () => {
await github.mergePr({
branchName: '',
id: pr.number,
strategy: 'merge-commit', // for coverage - has no effect on this test
}),
).toBeFalse();
});
Expand All @@ -3661,9 +3662,52 @@ describe('modules/platform/github/index', () => {
await github.mergePr({
branchName: '',
id: pr.number,
strategy: 'auto', // for coverage -- has not effect on this test
}),
).toBeFalse();
});

it('should warn if automergeStrategy is not supported', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope.put('/repos/some/repo/pulls/1234/merge').reply(200);
await github.initRepo({ repository: 'some/repo' });

const mergeResult = await github.mergePr({
id: 1234,
branchName: 'somebranch',
strategy: 'fast-forward',
});

expect(mergeResult).toBeTrue();
expect(logger.logger.warn).toHaveBeenCalledWith(
'Fast-forward merge strategy is not supported by Github. Falling back to merge strategy set for the repository.',
);
});

it('should use configured automergeStrategy', async () => {
const scope = httpMock.scope(githubApiHost);
initRepoMock(scope, 'some/repo');
scope.put('/repos/some/repo/pulls/1234/merge').reply(200);
await github.initRepo({ repository: 'some/repo' });

const mergeResult = await github.mergePr({
id: 1234,
branchName: 'somebranch',
strategy: 'rebase',
});

expect(mergeResult).toBeTrue();
expect(logger.logger.debug).toHaveBeenCalledWith(
{
options: {
body: { merge_method: 'rebase' },
},
url: 'repos/some/repo/pulls/1234/merge',
},
'mergePr',
);
});
});

describe('massageMarkdown(input)', () => {
Expand Down
12 changes: 8 additions & 4 deletions lib/modules/platform/github/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ import { repoFingerprint } from '../util';
import { normalizeNamePerEcosystem } from '../utils/github-alerts';
import { smartTruncate } from '../utils/pr-body';
import { remoteBranchExists } from './branch';
import { coerceRestPr, githubApi } from './common';
import { coerceRestPr, githubApi, mapMergeStartegy } from './common';
import {
enableAutoMergeMutation,
getIssuesQuery,
Expand Down Expand Up @@ -1847,6 +1847,7 @@ export async function reattemptPlatformAutomerge({
export async function mergePr({
branchName,
id: prNo,
strategy,
}: MergePRConfig): Promise<boolean> {
logger.debug(`mergePr(${prNo}, ${branchName})`);
const url = `repos/${
Expand All @@ -1861,9 +1862,12 @@ export async function mergePr({
}
let automerged = false;
let automergeResult: HttpResponse<unknown>;
if (config.mergeMethod) {
// This path is taken if we have auto-detected the allowed merge types from the repo
options.body.merge_method = config.mergeMethod;
const mergeStrategy = mapMergeStartegy(strategy) ?? config.mergeMethod;

if (mergeStrategy) {
// This path is taken if we have auto-detected the allowed merge types from the repo or
// automergeStrategy is configured by user
options.body.merge_method = mergeStrategy;
try {
logger.debug({ options, url }, `mergePr`);
automergeResult = await githubApi.putJson(url, options);
Expand Down
4 changes: 0 additions & 4 deletions lib/modules/platform/github/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,3 @@ When Renovate runs against repositories on `github.com`, and the environment var
<!-- prettier-ignore -->
!!! warning
We reverted the Package Registry Credentials feature to experimental mode, because users reported it's not working correctly with app tokens.

## Features awaiting implementation

- The `automergeStrategy` configuration option has not been implemented for this platform, and all values behave as if the value `auto` was used. Renovate will use the merge strategy configured in the GitHub repository itself, and this cannot be overridden yet

0 comments on commit 72a5af8

Please sign in to comment.