feat: combine branch automergeTypes

This deprecates branch-push and branch-merge-commit automergeTypes and replaces with “branch”, which has the same behaviour as the previous branch-push.

BREAKING CHANGE: branch-merge-commit automergeType behaviour is no longer supported, all branch automerges now use branch push approach.
This commit is contained in:
Rhys Arkins 2018-06-26 13:51:50 +02:00
parent 4d44752847
commit 8da5888ef6
14 changed files with 42 additions and 177 deletions

View file

@ -632,7 +632,7 @@ const options = [
{ {
name: 'automergeType', name: 'automergeType',
description: description:
'How to automerge - "branch-merge-commit", "branch-push", "pr-comment" or "pr". Branch support is GitHub-only', 'How to automerge - "branch", "pr", or "pr-comment". Branch support is GitHub-only',
type: 'string', type: 'string',
default: 'pr', default: 'pr',
}, },

View file

@ -133,6 +133,9 @@ function migrateConfig(config) {
migratedConfig.extends[i] = 'config:js-lib'; migratedConfig.extends[i] = 'config:js-lib';
} }
} }
} else if (key === 'automergeType' && val.startsWith('branch-')) {
isMigrated = true;
migratedConfig.automergeType = 'branch';
} else if (key === 'automergeMinor') { } else if (key === 'automergeMinor') {
isMigrated = true; isMigrated = true;
migratedConfig.minor = migratedConfig.minor || {}; migratedConfig.minor = migratedConfig.minor || {};

View file

@ -536,7 +536,7 @@ async function mergeBranch(branchName, mergeType) {
'Branch protection: Attempting to merge branch when push protection is enabled' 'Branch protection: Attempting to merge branch when push protection is enabled'
); );
} }
if (mergeType === 'branch-push') { if (mergeType === 'branch') {
const url = `repos/${config.repository}/git/refs/heads/${ const url = `repos/${config.repository}/git/refs/heads/${
config.baseBranch config.baseBranch
}`; }`;
@ -552,24 +552,7 @@ async function mergeBranch(branchName, mergeType) {
expandError(err), expandError(err),
`Error pushing branch merge for ${branchName}` `Error pushing branch merge for ${branchName}`
); );
throw new Error('branch-push failed'); throw new Error('Branch automerge failed');
}
} else if (mergeType === 'branch-merge-commit') {
const url = `repos/${config.repository}/merges`;
const options = {
body: {
base: config.baseBranch,
head: branchName,
},
};
try {
await get.post(url, options);
} catch (err) {
logger.info(
expandError(err),
`Error pushing branch merge for ${branchName}`
);
throw new Error('branch-merge-commit failed');
} }
} else { } else {
throw new Error(`Unsupported branch merge type: ${mergeType}`); throw new Error(`Unsupported branch merge type: ${mergeType}`);

View file

@ -18,7 +18,7 @@ async function getParentBranch(config) {
if ( if (
config.rebaseStalePrs || config.rebaseStalePrs ||
(config.rebaseStalePrs === null && (await platform.getRepoForceRebase())) || (config.rebaseStalePrs === null && (await platform.getRepoForceRebase())) ||
(config.automerge && config.automergeType === 'branch-push') (config.automerge && config.automergeType === 'branch')
) { ) {
const isBranchStale = await platform.isBranchStale(branchName); const isBranchStale = await platform.isBranchStale(branchName);
if (isBranchStale) { if (isBranchStale) {

View file

@ -10,6 +10,7 @@ exports[`config/migration migrateConfig(config, parentConfig) it migrates config
Object { Object {
"autodiscover": true, "autodiscover": true,
"automerge": false, "automerge": false,
"automergeType": "branch",
"baseBranches": Array [ "baseBranches": Array [
"next", "next",
], ],

View file

@ -17,6 +17,7 @@ describe('config/migration', () => {
automergeMinor: true, automergeMinor: true,
automergePatch: true, automergePatch: true,
upgradeInRange: true, upgradeInRange: true,
automergeType: 'branch-push',
baseBranch: 'next', baseBranch: 'next',
ignoreNodeModules: true, ignoreNodeModules: true,
node: { node: {

View file

@ -686,51 +686,7 @@ Object {
} }
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-merge-commit merge 1`] = ` exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch merge 1`] = `
Array [
Array [
"repos/some/repo",
],
Array [
"repos/some/repo/pulls?per_page=100&state=all",
Object {
"paginate": true,
},
],
Array [
"repos/some/repo/git/trees/master?recursive=true",
],
]
`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-merge-commit merge 2`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-merge-commit merge 3`] = `
Array [
Array [
"repos/some/repo/merges",
Object {
"body": Object {
"base": "master",
"head": "thebranchname",
},
},
],
]
`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-merge-commit merge 4`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-merge-commit merge 5`] = `
Array [
Array [
"repos/some/repo/git/refs/heads/thebranchname",
undefined,
],
]
`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-push merge 1`] = `
Array [ Array [
Array [ Array [
"repos/some/repo", "repos/some/repo",
@ -750,7 +706,7 @@ Array [
] ]
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-push merge 2`] = ` exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch merge 2`] = `
Array [ Array [
Array [ Array [
"repos/some/repo/git/refs/heads/master", "repos/some/repo/git/refs/heads/master",
@ -763,11 +719,11 @@ Array [
] ]
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-push merge 3`] = `Array []`; exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch merge 3`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-push merge 4`] = `Array []`; exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch merge 4`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch-push merge 5`] = ` exports[`platform/github mergeBranch(branchName, mergeType) should perform a branch merge 5`] = `
Array [ Array [
Array [ Array [
"repos/some/repo/git/refs/heads/thebranchname", "repos/some/repo/git/refs/heads/thebranchname",
@ -776,48 +732,9 @@ Array [
] ]
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 1`] = `[Error: branch-merge-commit failed]`; exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 1`] = `[Error: Branch automerge failed]`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 2`] = ` exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 2`] = `
Array [
Array [
"repos/some/repo",
],
Array [
"repos/some/repo/pulls?per_page=100&state=all",
Object {
"paginate": true,
},
],
Array [
"repos/some/repo/git/trees/master?recursive=true",
],
]
`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 3`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 4`] = `
Array [
Array [
"repos/some/repo/merges",
Object {
"body": Object {
"base": "master",
"head": "thebranchname",
},
},
],
]
`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 5`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-merge-commit throws 6`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 1`] = `[Error: branch-push failed]`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 2`] = `
Array [ Array [
Array [ Array [
"repos/some/repo", "repos/some/repo",
@ -837,7 +754,7 @@ Array [
] ]
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 3`] = ` exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 3`] = `
Array [ Array [
Array [ Array [
"repos/some/repo/git/refs/heads/master", "repos/some/repo/git/refs/heads/master",
@ -850,11 +767,11 @@ Array [
] ]
`; `;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 4`] = `Array []`; exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 4`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 5`] = `Array []`; exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 5`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch-push merge throws 6`] = `Array []`; exports[`platform/github mergeBranch(branchName, mergeType) should throw if branch merge throws 6`] = `Array []`;
exports[`platform/github mergeBranch(branchName, mergeType) should throw if unknown merge type 1`] = `[Error: Unsupported branch merge type: wrong-merge-type]`; exports[`platform/github mergeBranch(branchName, mergeType) should throw if unknown merge type 1`] = `[Error: Unsupported branch merge type: wrong-merge-type]`;

View file

@ -831,7 +831,7 @@ describe('platform/github', () => {
}); });
}); });
describe('mergeBranch(branchName, mergeType)', () => { describe('mergeBranch(branchName, mergeType)', () => {
it('should perform a branch-push merge', async () => { it('should perform a branch merge', async () => {
await initRepo({ await initRepo({
repository: 'some/repo', repository: 'some/repo',
token: 'token', token: 'token',
@ -854,14 +854,14 @@ describe('platform/github', () => {
})); }));
// deleteBranch // deleteBranch
get.delete.mockImplementationOnce(); get.delete.mockImplementationOnce();
await github.mergeBranch('thebranchname', 'branch-push'); await github.mergeBranch('thebranchname', 'branch');
expect(get.mock.calls).toMatchSnapshot(); expect(get.mock.calls).toMatchSnapshot();
expect(get.patch.mock.calls).toMatchSnapshot(); expect(get.patch.mock.calls).toMatchSnapshot();
expect(get.post.mock.calls).toMatchSnapshot(); expect(get.post.mock.calls).toMatchSnapshot();
expect(get.put.mock.calls).toMatchSnapshot(); expect(get.put.mock.calls).toMatchSnapshot();
expect(get.delete.mock.calls).toMatchSnapshot(); expect(get.delete.mock.calls).toMatchSnapshot();
}); });
it('should throw if branch-push merge throws', async () => { it('should throw if branch merge throws', async () => {
await initRepo({ await initRepo({
repository: 'some/repo', repository: 'some/repo',
token: 'token', token: 'token',
@ -874,51 +874,11 @@ describe('platform/github', () => {
}, },
})); }));
get.patch.mockImplementationOnce(() => { get.patch.mockImplementationOnce(() => {
throw new Error('branch-push failed'); throw new Error('branch failed');
}); });
let e; let e;
try { try {
await github.mergeBranch('thebranchname', 'branch-push'); await github.mergeBranch('thebranchname', 'branch');
} catch (err) {
e = err;
}
expect(e).toMatchSnapshot();
expect(get.mock.calls).toMatchSnapshot();
expect(get.patch.mock.calls).toMatchSnapshot();
expect(get.post.mock.calls).toMatchSnapshot();
expect(get.put.mock.calls).toMatchSnapshot();
expect(get.delete.mock.calls).toMatchSnapshot();
});
it('should perform a branch-merge-commit merge', async () => {
await initRepo({
repository: 'some/repo',
token: 'token',
}); // getBranchCommit
get.mockImplementationOnce(() => ({
body: {
object: {
sha: '1235',
},
},
}));
await github.mergeBranch('thebranchname', 'branch-merge-commit');
expect(get.mock.calls).toMatchSnapshot();
expect(get.patch.mock.calls).toMatchSnapshot();
expect(get.post.mock.calls).toMatchSnapshot();
expect(get.put.mock.calls).toMatchSnapshot();
expect(get.delete.mock.calls).toMatchSnapshot();
});
it('should throw if branch-merge-commit throws', async () => {
await initRepo({
repository: 'some/repo',
token: 'token',
});
get.post.mockImplementationOnce(() => {
throw new Error('branch-push failed');
});
let e;
try {
await github.mergeBranch('thebranchname', 'branch-merge-commit');
} catch (err) { } catch (err) {
e = err; e = err;
} }

View file

@ -20,20 +20,20 @@ describe('workers/branch/automerge', () => {
}); });
it('returns false if branch status is not success', async () => { it('returns false if branch status is not success', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('pending'); platform.getBranchStatus.mockReturnValueOnce('pending');
expect(await tryBranchAutomerge(config)).toBe('no automerge'); expect(await tryBranchAutomerge(config)).toBe('no automerge');
}); });
it('returns branch status error if branch status is failure', async () => { it('returns branch status error if branch status is failure', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('failure'); platform.getBranchStatus.mockReturnValueOnce('failure');
expect(await tryBranchAutomerge(config)).toBe('branch status error'); expect(await tryBranchAutomerge(config)).toBe('branch status error');
}); });
it('returns false if PR exists', async () => { it('returns false if PR exists', async () => {
platform.getBranchPr.mockReturnValueOnce({}); platform.getBranchPr.mockReturnValueOnce({});
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('success'); platform.getBranchStatus.mockReturnValueOnce('success');
expect(await tryBranchAutomerge(config)).toBe( expect(await tryBranchAutomerge(config)).toBe(
'automerge aborted - PR exists' 'automerge aborted - PR exists'
@ -41,7 +41,7 @@ describe('workers/branch/automerge', () => {
}); });
it('returns false if automerge fails', async () => { it('returns false if automerge fails', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('success'); platform.getBranchStatus.mockReturnValueOnce('success');
platform.mergeBranch.mockImplementationOnce(() => { platform.mergeBranch.mockImplementationOnce(() => {
throw new Error('merge error'); throw new Error('merge error');
@ -50,7 +50,7 @@ describe('workers/branch/automerge', () => {
}); });
it('returns true if automerge succeeds', async () => { it('returns true if automerge succeeds', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('success'); platform.getBranchStatus.mockReturnValueOnce('success');
expect(await tryBranchAutomerge(config)).toBe('automerged'); expect(await tryBranchAutomerge(config)).toBe('automerged');
}); });

View file

@ -59,16 +59,16 @@ describe('workers/branch/parent', () => {
expect(res.parentBranch).toBe(undefined); expect(res.parentBranch).toBe(undefined);
expect(platform.deleteBranch.mock.calls.length).toBe(1); expect(platform.deleteBranch.mock.calls.length).toBe(1);
}); });
it('returns branchName if automerge branch-push and not stale', async () => { it('returns branchName if automerge branch and not stale', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.branchExists.mockReturnValue(true); platform.branchExists.mockReturnValue(true);
const res = await getParentBranch(config); const res = await getParentBranch(config);
expect(res.parentBranch).toBe(config.branchName); expect(res.parentBranch).toBe(config.branchName);
}); });
it('returns undefined if automerge branch-push and stale', async () => { it('returns undefined if automerge branch and stale', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.branchExists.mockReturnValue(true); platform.branchExists.mockReturnValue(true);
platform.isBranchStale.mockReturnValueOnce(true); platform.isBranchStale.mockReturnValueOnce(true);
const res = await getParentBranch(config); const res = await getParentBranch(config);

View file

@ -299,7 +299,7 @@ describe('workers/pr', () => {
}); });
it('should create PR if branch tests failed', async () => { it('should create PR if branch tests failed', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
config.branchAutomergeFailureMessage = 'branch status error'; config.branchAutomergeFailureMessage = 'branch status error';
platform.getBranchStatus.mockReturnValueOnce('failure'); platform.getBranchStatus.mockReturnValueOnce('failure');
const pr = await prWorker.ensurePr(config); const pr = await prWorker.ensurePr(config);
@ -307,7 +307,7 @@ describe('workers/pr', () => {
}); });
it('should create PR if branch automerging failed', async () => { it('should create PR if branch automerging failed', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('success'); platform.getBranchStatus.mockReturnValueOnce('success');
config.forcePr = true; config.forcePr = true;
const pr = await prWorker.ensurePr(config); const pr = await prWorker.ensurePr(config);
@ -315,7 +315,7 @@ describe('workers/pr', () => {
}); });
it('should return null if branch automerging not failed', async () => { it('should return null if branch automerging not failed', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('pending'); platform.getBranchStatus.mockReturnValueOnce('pending');
platform.getBranchLastCommitTime.mockReturnValueOnce(new Date()); platform.getBranchLastCommitTime.mockReturnValueOnce(new Date());
const pr = await prWorker.ensurePr(config); const pr = await prWorker.ensurePr(config);
@ -323,7 +323,7 @@ describe('workers/pr', () => {
}); });
it('should not return null if branch automerging taking too long', async () => { it('should not return null if branch automerging taking too long', async () => {
config.automerge = true; config.automerge = true;
config.automergeType = 'branch-push'; config.automergeType = 'branch';
platform.getBranchStatus.mockReturnValueOnce('pending'); platform.getBranchStatus.mockReturnValueOnce('pending');
platform.getBranchLastCommitTime.mockReturnValueOnce( platform.getBranchLastCommitTime.mockReturnValueOnce(
new Date('2018-01-01') new Date('2018-01-01')

View file

@ -524,7 +524,7 @@ This feature supports simple caret (`^`) and tilde (`~`) ranges only, like `^1.0
This field is defaulted to `null` because it has a potential to create a lot of noise and additional builds to your repository. If you enable it to true, it means each Renovate branch will be updated whenever the base branch has changed. If enabled, this also means that whenever a Renovate PR is merged (whether by automerge or manually via GitHub web) then any other existing Renovate PRs will then need to get rebased and retested. This field is defaulted to `null` because it has a potential to create a lot of noise and additional builds to your repository. If you enable it to true, it means each Renovate branch will be updated whenever the base branch has changed. If enabled, this also means that whenever a Renovate PR is merged (whether by automerge or manually via GitHub web) then any other existing Renovate PRs will then need to get rebased and retested.
If you set it to `false` then that will take precedence - it means Renovate will ignore if you have configured the repository for "Require branches to be up to date before merging" in Branch Protection. However if you have configured it to `false` _and_ configured `branch-push` automerge then Renovate will still rebase as necessary for that. If you set it to `false` then that will take precedence - it means Renovate will ignore if you have configured the repository for "Require branches to be up to date before merging" in Branch Protection. However if you have configured it to `false` _and_ configured `branch` automerge then Renovate will still rebase as necessary for that.
## recreateClosed ## recreateClosed

View file

@ -71,7 +71,7 @@ Another example of a good candidate for automerging might be a database driver l
##### Branch automerging ##### Branch automerging
In the above suggestion of Pull Request automerging, you might still find it annoying if you receive GitHub Notifications for every PR that is created and merged. In that case, you could set `automergeType` to `branch-push`, which means Renovate will: In the above suggestion of Pull Request automerging, you might still find it annoying if you receive GitHub Notifications for every PR that is created and merged. In that case, you could set `automergeType` to `branch`, which means Renovate will:
- Create a new branch for testing - Create a new branch for testing
- Wait until after tests have completed - Wait until after tests have completed

View file

@ -86,7 +86,7 @@ For example, if you have `jest` or `mocha` as a dependency, and it has an upgrad
#### Branch automerging #### Branch automerging
Those of you familiar with GitHub might note that even if you automerge PRs, you are still going to get notifications (noise) anyway - one when the PR is created and another when it is merged. For this reason we recommend you consider setting `automergeType=branch-push` which will mean: Those of you familiar with GitHub might note that even if you automerge PRs, you are still going to get notifications (noise) anyway - one when the PR is created and another when it is merged. For this reason we recommend you consider setting `automergeType=branch` which will mean:
- Renovate first creates a branch and no PR - Renovate first creates a branch and no PR
- If tests pass, Renovate pushes a commit directly to `master` without PR - If tests pass, Renovate pushes a commit directly to `master` without PR
@ -116,7 +116,7 @@ Remember our running `eslint` example? Let's automerge it if all the linting upd
"groupName": "eslint", "groupName": "eslint",
"schedule": ["before 2am on monday"], "schedule": ["before 2am on monday"],
"automerge": true, "automerge": true,
"automergeType": "branch-push" "automergeType": "branch"
} }
] ]
``` ```