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

Allow setting merge method through labels #92

Merged
merged 5 commits into from
Aug 2, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ The following merge options are supported:
or [`squash`](https://help.github.com/en/articles/about-pull-request-merges#squash-and-merge-your-pull-request-commits)
(squash all commits into a single commit). The default option is `merge`.

- `MERGE_METHOD_LABELS`: Set to allow labels to determine the merge method.
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved
For example, `automerge=merge,autosquash=squash,autorebase=rebase`.

If no such label is present, use `MERGE_METHOD`.
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved

- `MERGE_METHOD_LABEL_REQUIRED`: Set to `"true"` to require one of the
`MERGE_METHOD_LABELS` to be set.

- `MERGE_COMMIT_MESSAGE`: The commit message to use when merging the pull
request into the base branch. Possible values are `automatic` (use GitHub's
default message), `pull-request-title` (use the pull request's title),
Expand Down
17 changes: 17 additions & 0 deletions lib/common.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ function createConfig(env = {}) {
};
}

function parseMergeMethodLabels(str) {
if (!str) {
return [];
}
return str.split(",").map(lm => {
const [label, method] = lm.split("=");
if (!label || !method) {
throw new Error(`Couldn't parse ${lm} as label=method`);
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved
}
return { label, method };
});
}

function parseMergeRemoveLabels(str, defaultValue) {
return (str == null ? defaultValue : str).split(",").map(s => s.trim());
}
Expand Down Expand Up @@ -97,6 +110,8 @@ function createConfig(env = {}) {
const mergeRetries = parsePositiveInt("MERGE_RETRIES", 6);
const mergeRetrySleep = parsePositiveInt("MERGE_RETRY_SLEEP", 5000);
const mergeDeleteBranch = env.MERGE_DELETE_BRANCH === "true";
const mergeMethodLabels = parseMergeMethodLabels(env.MERGE_METHOD_LABELS);
const mergeMethodLabelRequired = env.MERGE_METHOD_LABEL_REQUIRED === "true";

const updateLabels = parseMergeLabels(env.UPDATE_LABELS, "automerge");
const updateMethod = env.UPDATE_METHOD || "merge";
Expand All @@ -107,6 +122,8 @@ function createConfig(env = {}) {
mergeLabels,
mergeRemoveLabels,
mergeMethod,
mergeMethodLabels,
mergeMethodLabelRequired,
mergeForks,
mergeCommitMessage,
mergeCommitMessageRegex,
Expand Down
43 changes: 41 additions & 2 deletions lib/merge.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ async function merge(context, pullRequest) {
const {
octokit,
config: {
mergeMethod,
mergeMethod: defaultMergeMethod,
mergeMethodLabels,
mergeCommitMessage,
mergeCommitMessageRegex,
mergeFilterAuthor,
Expand Down Expand Up @@ -55,6 +56,11 @@ async function merge(context, pullRequest) {
}

const commitMessage = getCommitMessage(mergeCommitMessage, pullRequest);
const mergeMethod = getMergeMethod(
defaultMergeMethod,
mergeMethodLabels,
pullRequest
);
const merged = await tryMerge(
octokit,
pullRequest,
Expand Down Expand Up @@ -143,7 +149,12 @@ async function deleteBranch(octokit, pullRequest) {

function skipPullRequest(context, pullRequest) {
const {
config: { mergeForks, mergeLabels }
config: {
mergeForks,
mergeLabels,
mergeMethodLabelRequired,
mergeMethodLabels
}
} = context;

let skip = false;
Expand Down Expand Up @@ -181,6 +192,14 @@ function skipPullRequest(context, pullRequest) {
}
}

const numberMethodLabelsFound = mergeMethodLabels
.map(lm => labels.includes(lm.label))
.filter(x => x).length;
if (mergeMethodLabelRequired && numberMethodLabelsFound === 0) {
logger.info("Skipping PR merge, required merge method label missing");
skip = true;
}

return skip;
}

Expand Down Expand Up @@ -255,6 +274,26 @@ function tryMerge(
);
}

function getMergeMethod(defaultMergeMethod, mergeMethodLabels, pullRequest) {
const foundMergeMethodLabels = pullRequest.labels.flatMap(l =>
mergeMethodLabels.filter(ml => ml.label === l.name)
);
if (foundMergeMethodLabels.length > 0) {
const first = foundMergeMethodLabels[0];
if (foundMergeMethodLabels.length > 1) {
logger.info(
`Discovered multiple merge method labels, will merge with method ${first.method}`
);
michaelbeaumont marked this conversation as resolved.
Show resolved Hide resolved
} else {
logger.info(
`Discovered ${first.label}, will merge with method ${first.method}`
);
}
return first.method;
}
return defaultMergeMethod;
}

function getCommitMessage(mergeCommitMessage, pullRequest) {
if (mergeCommitMessage === "automatic") {
return undefined;
Expand Down
2 changes: 2 additions & 0 deletions test/common.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@ test("createConfig", () => {
blocking: [],
required: []
},
mergeMethodLabels: [],
mergeMethodLabelRequired: false,
mergeForks: true,
mergeCommitMessage: "automatic",
mergeCommitMessageRegex: "",
Expand Down
84 changes: 82 additions & 2 deletions test/merge.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,13 @@ const { merge } = require("../lib/merge");
const { createConfig } = require("../lib/common");
const { pullRequest } = require("./common");

let octokit;
let octokit, mergeMethod;

beforeEach(() => {
mergeMethod = undefined;
octokit = {
pulls: {
merge: jest.fn()
merge: jest.fn(({ merge_method }) => (mergeMethod = merge_method))
}
};
});
Expand Down Expand Up @@ -109,3 +110,82 @@ test("MERGE_FILTER_AUTHOR when set but do not match current author should not me
// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(false);
});

test("Merge method can be set by env variable", async () => {
// GIVEN
const pr = pullRequest();

const config = createConfig({
MERGE_METHOD: "rebase"
});

// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(true);
expect(mergeMethod).toEqual("rebase");
});

test("Merge method can be set by a merge method label", async () => {
// GIVEN
const pr = pullRequest();
pr.labels = [{ name: "autosquash" }, { name: "reallyautomerge" }];

const config = createConfig({
MERGE_METHOD_LABELS: "automerge=merge,autosquash=squash,autorebase=rebase",
MERGE_METHOD: "merge",
MERGE_LABELS: "reallyautomerge"
});

// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(true);
expect(mergeMethod).toEqual("squash");
});

test("Merge method can be required", async () => {
// GIVEN
const pr = pullRequest();
pr.labels = [{ name: "autosquash" }];

const config = createConfig({
MERGE_METHOD_LABELS: "automerge=merge,autosquash=squash,autorebase=rebase",
MERGE_METHOD_LABEL_REQUIRED: "true",
MERGE_METHOD: "merge",
MERGE_LABELS: ""
});

// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(true);
expect(mergeMethod).toEqual("squash");
});

test("Missing require merge method skips PR", async () => {
// GIVEN
const pr = pullRequest();
pr.labels = [{ name: "mergeme" }];

const config = createConfig({
MERGE_METHOD_LABELS: "automerge=merge,autosquash=squash,autorebase=rebase",
MERGE_METHOD_LABEL_REQUIRED: "true",
MERGE_METHOD: "merge",
MERGE_LABELS: "mergeme"
});

// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(false);
});

test("Merge method doesn't have to be required", async () => {
// GIVEN
const pr = pullRequest();
pr.labels = [{ name: "mergeme" }];

const config = createConfig({
MERGE_METHOD_LABELS: "automerge=merge,autosquash=squash,autorebase=rebase",
MERGE_METHOD_LABEL_REQUIRED: "false",
MERGE_METHOD: "merge",
MERGE_LABELS: "mergeme"
});

// WHEN
expect(await merge({ config, octokit }, pr)).toEqual(true);
expect(mergeMethod).toEqual("merge");
});