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

Squash on green support #161

Merged
merged 4 commits into from
Sep 11, 2020
Merged
Show file tree
Hide file tree
Changes from all 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
35 changes: 24 additions & 11 deletions org/markAsMergeOnGreen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,23 @@ import { danger } from "danger"
import { IssueComment, PullRequestReview } from "github-webhook-event-types"
import { IssueCommentIssue } from "github-webhook-event-types/source/IssueComment"

export const labelMap = {
"#mergeongreen": {
name: "Merge On Green",
color: "247A38",
description: "A label to indicate that Peril should merge this PR when all statuses are green",
mergeMethod: "merge",
commitGenerator: (prNumber: number) => `Merge pull request #${prNumber} by Peril`,
},
"#squashongreen": {
name: "Squash On Green",
color: "247A38",
description: "A label to indicate that Peril should squash-merge this PR when all statuses are green",
mergeMethod: "squash",
commitGenerator: undefined, // defaults to GitHub's generated commit messages
},
} as const

/** If a comment to an issue contains "Merge on Green", apply a label for it to be merged when green. */
export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestReview) => {
const api = danger.github.api
Expand Down Expand Up @@ -42,15 +59,17 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe
}

// Don't do any work unless we have to
const keywords = ["#mergeongreen"]
const match = keywords.find(k => text.toLowerCase().includes(k))
const keywords = Object.keys(labelMap)
const match = keywords.find(k => text.toLowerCase().includes(k)) as keyof typeof labelMap | undefined
if (!match) {
return console.log(`Did not find any of the merging phrases in the comment beginning ${text.substring(0, 12)}.`)
}

const label = labelMap[match]

// Check to see if the label has already been set
if (issue.labels.find(l => l.name === "Merge On Green")) {
return console.log("Already has Merge on Green")
if (issue.labels.find(l => l.name === label.name)) {
return console.log("Already has Merge on Green-type label")
}

// Check for org access, so that some rando doesn't
Expand All @@ -64,12 +83,6 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe
return console.log("Sender does not have permission to merge")
}

// Let's people know that it will be merged
const label = {
name: "Merge On Green",
color: "247A38",
description: "A label to indicate that Peril should merge this PR when all statuses are green",
}
const repo = {
owner: org,
repo: issueCommentOrPrReview.repository.name,
Expand All @@ -78,7 +91,7 @@ export const rfc10 = async (issueCommentOrPrReview: IssueComment | PullRequestRe

console.log("Adding the label:", repo)
await danger.github.utils.createOrAddLabel(label, repo)
console.log("Updated the PR with a Merge on Green label")
console.log("Updated the PR with a Merge on Green-type label")
}

export default rfc10
13 changes: 8 additions & 5 deletions org/mergeOnGreen.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { danger } from "danger"
import { Status } from "github-webhook-event-types"
import { LabelLabel } from "github-webhook-event-types/source/Label"
import { labelMap } from "./markAsMergeOnGreen"
import { capitalize } from "lodash"

export const rfc10 = async (status: Status) => {
const api = danger.github.api
Expand Down Expand Up @@ -30,12 +31,14 @@ export const rfc10 = async (status: Status) => {
const issue = await api.issues.get({ owner, repo, number })

// Get the PR combined status
const mergeLabel = issue.data.labels.find((l: LabelLabel) => l.name === "Merge On Green")
const issueLabelNames = issue.data.labels.map(l => l.name)
const mergeLabel = Object.values(labelMap).find(label => issueLabelNames.includes(label.name))

if (!mergeLabel) {
return console.log("PR does not have Merge on Green")
return console.log("PR does not have Merge on Green-type label")
}

let commitTitle = `Merge pull request #${number} by Peril`
let commitTitle = mergeLabel.commitGenerator?.(number)

if (issue.data.title) {
// Strip any "@user =>" prefixes from the pr title
Expand All @@ -44,7 +47,7 @@ export const rfc10 = async (status: Status) => {
}

// Merge the PR
await api.pulls.merge({ owner, repo, number, commit_title: commitTitle })
await api.pulls.merge({ owner, repo, number, commit_title: commitTitle, merge_method: mergeLabel.mergeMethod })
console.log(`Merged Pull Request ${number}`)
}
}
Expand Down
10 changes: 5 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -22,20 +22,20 @@
"@types/jest": "^23.3.9",
"@types/jira-client": "^6.4.0",
"@types/lodash": "^4.14.121",
"@types/node": "^10.12.9",
"@types/node": "^14.6.4",
"@types/node-fetch": "^2.1.3",
"danger": "^7.0.9",
"danger-plugin-spellcheck": "^1.2.3",
"danger-plugin-yarn": "^1.3.0",
"github-webhook-event-types": "^1.2.1",
"husky": "^0.14.3",
"jest": "^24.3.0",
"jest": "^26.4.2",
"jira-client": "^6.4.1",
"lint-staged": "^7.2.2",
"prettier": "^1.15.2",
"ts-jest": "^24.0.0",
"prettier": "^2.1.1",
"ts-jest": "^26.3.0",
"ts-node": "^7.0.1",
"typescript": "^3.1.6"
"typescript": "^4.0.2"
},
"prettier": {
"printWidth": 120,
Expand Down
63 changes: 59 additions & 4 deletions tests/rfc_10.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,16 @@ describe("for adding the label", () => {
issue: { labels: [{ name: "Merge On Green" }], pull_request: {} },
...repo,
} as any)
expect(console.log).toBeCalledWith("Already has Merge on Green")
expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
})

it("bails when the issue already has merge on green", async () => {
await markAsMergeOnGreen({
comment: { body: "#squashongreen", user: { login: "danger" } },
issue: { labels: [{ name: "Squash On Green" }], pull_request: {} },
...repo,
} as any)
expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
})
})

Expand Down Expand Up @@ -96,10 +105,26 @@ describe("for adding the label", () => {
pull_request: pull_request,
...repo,
} as any)
expect(console.log).toBeCalledWith("Already has Merge on Green")
expect(console.log).toBeCalledWith("Already has Merge on Green-type label")
})
})

it("creates the label when the label doesn't exist on the repo", async () => {
dm.danger.github.api.orgs.checkMembership.mockReturnValueOnce(Promise.resolve({ data: {} }))
dm.danger.github.api.issues.getLabels.mockReturnValueOnce(Promise.resolve({ data: [] }))

await markAsMergeOnGreen({
comment: {
body: "#squashongreen",
user: { sender: { login: "orta" } },
},
issue: { labels: [], pull_request: {} },
...repo,
} as any)

expect(dm.danger.github.utils.createOrAddLabel).toBeCalled()
})

it("creates the label when the label doesn't exist on the repo", async () => {
dm.danger.github.api.orgs.checkMembership.mockReturnValueOnce(Promise.resolve({ data: {} }))
dm.danger.github.api.issues.getLabels.mockReturnValueOnce(Promise.resolve({ data: [] }))
Expand Down Expand Up @@ -157,10 +182,10 @@ describe("for handling merging when green", () => {
commit: { sha: "123abc" },
} as any)

expect(console.log).toBeCalledWith("PR does not have Merge on Green")
expect(console.log).toBeCalledWith("PR does not have Merge on Green-type label")
})

it("triggers a PR merge when there is a merge on green label", async () => {
it("triggers a PR merge when there is a Merge on Green label", async () => {
// Has the right status
dm.danger.github.api.repos.getCombinedStatusForRef.mockReturnValueOnce(
Promise.resolve({ data: { state: "success" } })
Expand All @@ -185,6 +210,36 @@ describe("for handling merging when green", () => {
number: 1,
owner: "danger",
repo: "doggo",
merge_method: "merge",
})
})

it("triggers a PR squash when there is a Squash on Green label", async () => {
// Has the right status
dm.danger.github.api.repos.getCombinedStatusForRef.mockReturnValueOnce(
Promise.resolve({ data: { state: "success" } })
)

// Gets a corresponding issue
dm.danger.github.api.search.issues.mockReturnValueOnce(Promise.resolve({ data: { items: [{ number: 1 }] } }))

// Returns an issue without the merge on green label
dm.danger.github.api.issues.get.mockReturnValueOnce(
Promise.resolve({ data: { labels: [{ name: "Squash On Green" }] } })
)

await mergeOnGreen({
state: "success",
repository: { owner: { login: "danger" }, name: "doggo" },
commit: { sha: "123abc" },
} as any)

expect(dm.danger.github.api.pulls.merge).toBeCalledWith({
commit_title: undefined,
number: 1,
owner: "danger",
repo: "doggo",
merge_method: "squash",
})
})
})
Loading