Remove code which allows merging of PRs (#52)

This commit is contained in:
Jake Bailey 2024-05-14 20:43:44 -07:00 коммит произвёл GitHub
Родитель 0184e6bcdf
Коммит b2ddb4d664
Не найден ключ, соответствующий данной подписи
Идентификатор ключа GPG: B5690EEEBB952194
15 изменённых файлов: 11 добавлений и 507 удалений

8
.github/workflows/ci.yml поставляемый
Просмотреть файл

@ -3,7 +3,13 @@ on: pull_request
jobs:
test:
runs-on: ubuntu-latest
strategy:
fail-fast: false
matrix:
os:
- ubuntu-latest
- windows-latest
runs-on: ${{ matrix.os }}
steps:
- uses: actions/checkout@44c2b7a8a4ea60a981eaca3cf939b5f4305c123b # v4.1.5
- uses: actions/setup-node@60edb5dd545a775178f52524783378180af0d1f8 # v4.0.2

Просмотреть файл

@ -9,7 +9,6 @@
- Adds a label for any new PR by a core team member
- Assigns a core team member to their own PR if no-one is assigned
- Allows merging on green when a PR's statuses are all green and there is a label "Merge on Green" on the PR
#### Issues

28
package-lock.json сгенерированный
Просмотреть файл

@ -11,8 +11,7 @@
"dependencies": {
"@azure/functions": "^4.4.0",
"@octokit/rest": "^20.1.1",
"@octokit/webhooks-methods": "^4.1.0",
"minimatch": "^9.0.4"
"@octokit/webhooks-methods": "^4.1.0"
},
"devDependencies": {
"@octokit/webhooks-types": "^7.5.1",
@ -1558,21 +1557,14 @@
"node_modules/balanced-match": {
"version": "1.0.2",
"resolved": "https://registry.npmjs.org/balanced-match/-/balanced-match-1.0.2.tgz",
"integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw=="
"integrity": "sha512-3oSeUO0TMV67hN1AmbXsK4yaqU7tjiHlbxRDZOpH0KW9+CeX4bRAaX0Anxt0tx2MrpRpWwQaPwIlISEJhYU5Pw==",
"dev": true
},
"node_modules/before-after-hook": {
"version": "2.2.3",
"resolved": "https://registry.npmjs.org/before-after-hook/-/before-after-hook-2.2.3.tgz",
"integrity": "sha512-NzUnlZexiaH/46WDhANlyR2bXRopNg4F/zuSA3OpZnllCUgRaOF2znDioDWrmbNVsuZk6l9pMquQB38cfBZwkQ=="
},
"node_modules/brace-expansion": {
"version": "2.0.1",
"resolved": "https://registry.npmjs.org/brace-expansion/-/brace-expansion-2.0.1.tgz",
"integrity": "sha512-XnAIvQ8eM+kC6aULx6wuQiwVsnzsi9d3WxzV3FpWTGA19F621kwdbsAcFKXgKUHZWsy+mY6iL1sHTxWEFCytDA==",
"dependencies": {
"balanced-match": "^1.0.0"
}
},
"node_modules/braces": {
"version": "3.0.2",
"resolved": "https://registry.npmjs.org/braces/-/braces-3.0.2.tgz",
@ -3535,20 +3527,6 @@
"node": ">=6"
}
},
"node_modules/minimatch": {
"version": "9.0.4",
"resolved": "https://registry.npmjs.org/minimatch/-/minimatch-9.0.4.tgz",
"integrity": "sha512-KqWh+VchfxcMNRAJjj2tnsSJdNbHsVgnkBhTNrW7AjVo6OvLtxw8zfT9oLw1JSohlFzJ8jCoTgaoXvJ+kHt6fw==",
"dependencies": {
"brace-expansion": "^2.0.1"
},
"engines": {
"node": ">=16 || 14 >=14.17"
},
"funding": {
"url": "https://github.com/sponsors/isaacs"
}
},
"node_modules/minimist": {
"version": "1.2.8",
"resolved": "https://registry.npmjs.org/minimist/-/minimist-1.2.8.tgz",

Просмотреть файл

@ -18,8 +18,7 @@
"dependencies": {
"@azure/functions": "^4.4.0",
"@octokit/rest": "^20.1.1",
"@octokit/webhooks-methods": "^4.1.0",
"minimatch": "^9.0.4"
"@octokit/webhooks-methods": "^4.1.0"
},
"devDependencies": {
"@octokit/webhooks-types": "^7.5.1",

Просмотреть файл

@ -3,7 +3,6 @@ import { HttpResponseInit, InvocationContext } from "@azure/functions"
import { createGitHubClient } from "./util/createGitHubClient"
import { Octokit } from "@octokit/rest"
import { sha } from "./sha"
import { mergeThroughCodeOwners } from "./checks/mergeThroughCodeOwners"
import { addReprosLabelOnComments } from "./checks/addReprosLabel"
import { Logger } from "./util/logger"
@ -20,11 +19,6 @@ export const anyRepoHandleIssueCommentPayload = async (payload: IssueCommentEven
return fn(api, payload, context)
}
// Making this one whitelisted to the website for now
if (payload.repository.name === "TypeScript-Website") {
await run("Checking if we should merge from codeowners", mergeThroughCodeOwners)
}
if (payload.repository.name === "TypeScript") {
await run("Checking if we should add the repros label", addReprosLabelOnComments)
}

Просмотреть файл

@ -2,7 +2,6 @@ import { StatusEvent } from "@octokit/webhooks-types"
import { createGitHubClient } from "./util/createGitHubClient"
import { Octokit } from "@octokit/rest"
import { sha } from "./sha"
import { mergeOnGreen } from "./checks/mergeOnGreen"
import { HttpResponseInit, InvocationContext } from "@azure/functions"
import { Logger } from "./util/logger"
@ -16,11 +15,6 @@ export const anyRepoHandleStatusUpdate = async (payload: StatusEvent, context: I
return fn(api, payload, context)
}
// Run checks
if (payload.repository.name === "TypeScript-Website") {
await run("Checking For Merge on Green", mergeOnGreen)
}
return {
status: 200,
headers: { sha: sha },

Просмотреть файл

@ -1,83 +0,0 @@
import { createMockGitHubClient } from "../util/tests/createMockGitHubClient"
import { getFakeLogger } from "../util/tests/createMockContext"
import { mergeOnGreen } from "./mergeOnGreen"
describe("for handling merging when green", () => {
it("bails when its not a success", async () => {
const { api } = createMockGitHubClient()
const logger = getFakeLogger()
await mergeOnGreen(api, { state: "fail" } as any, logger)
expect(logger.info).toBeCalled()
})
it("bails when the whole status is not a success", async () => {
const { mockAPI, api } = createMockGitHubClient()
const logger = getFakeLogger()
mockAPI.checks.listForRef.mockResolvedValueOnce({ data: { check_runs: [{ conclusion: "FAILED" }]}})
const webhook = {
state: "success",
repository: { owner: { login: "danger" }, name: "doggo" },
commit: { sha: "123abc" },
} as any
await mergeOnGreen(api, webhook, logger)
expect(logger.info).toBeCalledWith("Not all statuses are green")
})
it("does nothing when the PR does not have merge on green", async () => {
const { mockAPI, api } = createMockGitHubClient()
const logger = getFakeLogger()
// Says al CI statuses are green
mockAPI.checks.listForRef.mockResolvedValueOnce({ data: { check_runs: [{ conclusion: "success" }]}})
// Gets a corresponding issue
mockAPI.search.issuesAndPullRequests.mockResolvedValueOnce({ data: { items: [{ number: 1 }] } })
// Returns an issue without the merge on green label
mockAPI.issues.get.mockResolvedValueOnce({ data: { labels: [{ name: "Dog Snoozer" }] } })
const webhook = {
state: "success",
repository: { owner: { login: "danger" }, name: "doggo" },
commit: { sha: "123abc" },
} as any
await mergeOnGreen(api, webhook, logger)
expect(logger.info).toBeCalledWith("PR 1 does not have Merge on Green")
})
it("triggers a PR merge when there is a merge on green label", async () => {
const { mockAPI, api } = createMockGitHubClient()
const logger = getFakeLogger()
// Says al CI statuses are green
mockAPI.checks.listForRef.mockResolvedValueOnce({ data: { check_runs: [{ conclusion: "success" }]}})
// Gets a corresponding issue
mockAPI.search.issuesAndPullRequests.mockResolvedValueOnce({ data: { items: [{ number: 1 }] } })
// Returns an issue without the merge on green label
mockAPI.issues.get.mockResolvedValueOnce({ data: { labels: [{ name: "Merge On Green" }] } })
const webhook = {
state: "success",
repository: { owner: { login: "danger" }, name: "doggo" },
commit: { sha: "123abc" },
} as any
await mergeOnGreen(api, webhook, logger)
expect(mockAPI.pulls.merge).toBeCalledWith({
commit_title: "Merge pull request #1 by microsoft/typescript-repos-automation",
pull_number: 1,
owner: "danger",
repo: "doggo",
})
})
})

Просмотреть файл

@ -1,56 +0,0 @@
import { StatusEvent } from "@octokit/webhooks-types"
import { Octokit } from "@octokit/rest"
import { Logger } from "../util/logger"
/**
* If the PR comes from a core contributor, set themselves to be the assignee
* if one isn't set during the creation of the PR.
*/
export const mergeOnGreen = async (api: Octokit, payload: StatusEvent, logger: Logger) => {
if (payload.state !== "success") {
return logger.info(`Not a successful state - got ${payload.state}`)
}
// Check to see if all other statuses on the same commit are also green. E.g. is this the last green.
const owner = payload.repository.owner.login
const repo = payload.repository.name
const status = await api.checks.listForRef({ owner, repo, ref: payload.commit.sha })
if (status.data.check_runs.every(c => c.conclusion !== "success")) {
return logger.info("Not all statuses are green")
}
// See https://github.com/maintainers/early-access-feedback/issues/114 for more context on getting a PR from a SHA
const repoString = payload.repository.full_name
const searchResponse = await api.search.issuesAndPullRequests({ q: `${payload.commit.sha} type:pr is:open repo:${repoString}` })
// https://developer.github.com/v3/search/#search-issues
const prsWithCommit = searchResponse.data.items.map((i: any) => i.number) as number[]
for (const number of prsWithCommit) {
// Get the PR labels
const issue = await api.issues.get({ owner, repo, issue_number: number })
// Get the PR combined status
const mergeLabel = issue.data.labels.find(l => {
const name = typeof l === "string" ? l : l.name;
return name?.toLowerCase() === "merge on green"
})
if (!mergeLabel) {
return logger.info(`PR ${number} does not have Merge on Green`)
} else {
logger.info(`Merging PR ${number} via Merge on Green`)
}
let commitTitle = `Merge pull request #${number} by microsoft/typescript-repos-automation`
if (issue.data.title) {
// Strip any "@user =>" prefixes from the pr title
const prTitle = issue.data.title.replace(/@(\w|-)+\s+=>\s+/, "")
commitTitle = `${prTitle} (#${number})`
}
// Merge the PR
await api.pulls.merge({ owner, repo, pull_number: number, commit_title: commitTitle })
logger.info(`Merged Pull Request ${number}`)
}
}

Просмотреть файл

@ -1,93 +0,0 @@
jest.mock("../util/getContents")
import { createMockGitHubClient, getPRFixture } from "../util/tests/createMockGitHubClient"
import { getFakeLogger } from "../util/tests/createMockContext"
import { mergeThroughCodeOwners, mergePhrase } from "./mergeThroughCodeOwners"
import { getContents } from "../util/getContents"
const genericWebhook = {
comment: {
body: mergePhrase,
user: {
login: "orta",
},
},
issue: {
number: 1234,
},
repository: {
owner: {
login: "microsoft",
},
name: "TypeScript-Website",
},
}
describe("for handling merging when green", () => {
it("bails when the keyword aren't used", async () => {
const { api } = createMockGitHubClient()
const logger = getFakeLogger()
await mergeThroughCodeOwners(api, { comment: { body: "Good Joke" } } as any, logger)
expect(logger.info).toBeCalledWith(
"Issue comment did not include 'ready to merge', skipping merge through code owners checks"
)
})
it("handles the phrase in an issue", async () => {
const { api, mockAPI } = createMockGitHubClient()
mockAPI.pulls.get.mockRejectedValue(new Error("not found"))
const logger = getFakeLogger()
await mergeThroughCodeOwners(api, genericWebhook as any, logger)
expect(logger.info).toBeCalledWith("Comment was in an issue")
})
it("handles the phrase in an pr", async () => {
const { api, mockAPI } = createMockGitHubClient()
const logger = getFakeLogger()
const pr = getPRFixture("api-pr-closed")
// @ts-ignore
getContents.mockReturnValue("/hello.txt @orta")
mockAPI.repos.getCombinedStatusForRef.mockResolvedValue({ data: { state: "success" } })
// Getting the PR form the API
mockAPI.pulls.get.mockResolvedValue({ data: pr })
// Getting the files
mockAPI.pulls.listFiles.endpoint.merge.mockResolvedValue({})
mockAPI.paginate.mockResolvedValue([{ filename: "/hello.txt" }])
await mergeThroughCodeOwners(api, genericWebhook as any, logger)
expect(logger.info).toBeCalledWith("Accepting as reasonable to merge")
})
it.skip("handles a complex regex and owner group in an pr", async () => {
const { api, mockAPI } = createMockGitHubClient()
const logger = getFakeLogger()
const pr = getPRFixture("api-pr-closed")
// @ts-ignore
getContents.mockReturnValue(`
/packages/playground-examples/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
/packages/tsconfig-reference/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
/packages/typescriptlang-org/src/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
`)
// Getting the PR form the API
mockAPI.pulls.get.mockResolvedValue({ data: pr })
// Getting the files
mockAPI.pulls.listFiles.endpoint.merge.mockResolvedValue({})
mockAPI.paginate.mockResolvedValue([
{ filename: "/packages/playground-examples/copy/ja/TypeScript/Primitives/Any.ts" },
])
mockAPI.repos.getCombinedStatusForRef.mockResolvedValue({ data: { state: "success" } })
await mergeThroughCodeOwners(api, genericWebhook as any, logger)
expect(logger.info).toBeCalledWith("Accepting as reasonable to merge")
})
})

Просмотреть файл

@ -1,43 +0,0 @@
import { IssueCommentEvent } from "@octokit/webhooks-types"
import { Octokit, RestEndpointMethodTypes } from "@octokit/rest"
import { hasAccessToMergePRs } from "../pr_meta/hasAccessToMergePR"
import { mergeOrAddMergeLabel } from "../pr_meta/mergeOrAddMergeLabel"
import { Logger } from "../util/logger"
export const mergePhrase = "ready to merge"
/**
* Allow someone to declare a PR should be merged if they have access rights via code owners
*/
export const mergeThroughCodeOwners = async (api: Octokit, payload: IssueCommentEvent, logger: Logger) => {
if (!payload.comment.body.toLowerCase().includes(mergePhrase)) {
return logger.info(`Issue comment did not include '${mergePhrase}', skipping merge through code owners checks`)
}
// Grab the correlating PR
let pull: RestEndpointMethodTypes["pulls"]["get"]["response"]["data"]
try {
const response = await api.pulls.get({
owner: payload.repository.owner.login,
repo: payload.repository.name,
pull_number: payload.issue.number,
})
pull = response.data
} catch (error) {
return logger.info(`Comment was in an issue`)
}
const canMerge = await hasAccessToMergePRs(payload.comment.user.login, api, pull, logger)
if (!canMerge) {
return // it logs in the function above
}
logger.info("Looks good to merge")
await mergeOrAddMergeLabel(
api,
{ pull_number: pull.number, repo: pull.base.repo.name, owner: pull.base.repo.owner.login },
pull.head.sha,
logger
)
}

Просмотреть файл

@ -1,47 +0,0 @@
import { getFilesNotOwnedByCodeOwner, getCodeOwnersInfo } from "./hasAccessToMergePR"
it("matches simple files", () => {
const codeOwnersText = `
/a/b @orta
`
const files = ["/a/b"]
const result = getFilesNotOwnedByCodeOwner("orta", files, getCodeOwnersInfo(codeOwnersText))
expect(result).toEqual([])
})
it("matches simple files", () => {
const codeOwnersText = `
/a/b/c/* @orta
/a/b/d/* @orta
/b/c @hayes
`
const files = ["/a/b", "/a/b/c/deff", "/b/c"]
const result = getFilesNotOwnedByCodeOwner("orta", files, getCodeOwnersInfo(codeOwnersText))
expect(result).toEqual(["/a/b", "/b/c"])
})
it("matches simple files", () => {
const codeOwnersText = `
/packages/**/ja/** @hayes
/a/b/d/* @orta
/b/c @hayes
`
const files = ["/packages/playground-examples/copy/ja/TypeScript/Type Primitives/Built-in Utility Types.ts ", "/b/c"]
const result = getFilesNotOwnedByCodeOwner("hayes", files, getCodeOwnersInfo(codeOwnersText))
expect(result).toEqual([])
})
it("matches real life files", () => {
const codeOwnersText = `
/packages/playground-examples/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
/packages/tsconfig-reference/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
/packages/typescriptlang-org/src/copy/ja/** @sasurau4 @Quramy @Naturalclar @Takepepe @orta
`
const files = ["/packages/playground-examples/copy/ja/TypeScript/Type Primitives/Built-in Utility Types.ts ", "/b/c"]
const result = getFilesNotOwnedByCodeOwner("orta", files, getCodeOwnersInfo(codeOwnersText))
expect(result).toEqual(["/b/c"])
})

Просмотреть файл

@ -1,111 +0,0 @@
import { Octokit, RestEndpointMethodTypes } from "@octokit/rest"
import { getContents } from "../util/getContents"
import { minimatch } from "minimatch"
import { Logger } from "../util/logger"
type PR = RestEndpointMethodTypes["pulls"]["get"]["response"]["data"];
/**
* Checks if a user has access to merge via a comment
*
* @param commenterLogin the username of who we're checking if they can merge
* @param octokit authed api for github
* @param pr the JSON from a get request for a PR
* @param logger logs
*/
export const hasAccessToMergePRs = async (commenterLogin: string, octokit: Octokit, pr: PR, logger: Logger) => {
const codeownersText = await getCodeOwnersFileForRepo(octokit, pr)
if (codeownersText === "") {
logger.info("Skipping because there is no code-owners file in the repo")
return false
}
const changedFiles = await getPRChangedFiles(octokit, pr)
const codeOwners = getCodeOwnersInfo(codeownersText)
const filesWhichArentOwned = getFilesNotOwnedByCodeOwner(commenterLogin, changedFiles, codeOwners)
if (filesWhichArentOwned.length > 0) {
logger.info(`- Bailing because not all files for ${commenterLogin} were covered by the codeowners for this review`)
logger.info(` Missing: ${filesWhichArentOwned.join(", ")}`)
return false
} else {
logger.info("Accepting as reasonable to merge")
return true
}
}
// async function comm
export function getFilesNotOwnedByCodeOwner(commenterLogin: string, files: string[], codeOwners: CodeOwner[]) {
const atUser = "@" + commenterLogin
const activeCodeOwners = codeOwners.filter(c => c.usernames.includes(atUser))
if (activeCodeOwners.length == 0) {
// Couldn't find anything, so return all the files
return files
}
// Make a copy of all matches, then loop through known codeowners removing anything which passes
let matched = [...files]
activeCodeOwners.forEach(owners => {
matched.forEach((file, index) => {
if (minimatch(file, owners.path)) {
matched.splice(index, 1)
}
})
})
return matched
}
async function getPRChangedFiles(octokit: Octokit, webhook: PR) {
// https://developer.github.com/v3/pulls/#list-pull-requests-files
const repo = webhook.base.repo
const options = octokit.pulls.listFiles.endpoint.merge({
owner: repo.owner.login,
repo: repo.name,
pull_number: webhook.number,
})
const files = await octokit.paginate<RestEndpointMethodTypes["pulls"]["listFiles"]["response"]["data"][number]>(options)
const fileStrings = files.map(f => `/${f.filename}`)
return fileStrings
}
const getCodeOwnersFileForRepo = (api: Octokit, webhook: PR) => {
const repo = webhook.base.repo
try {
return getContents(api, { owner: repo.owner.login, repo: repo.name, path: ".github/CODEOWNERS" })
} catch (error) {
return ""
}
}
type CodeOwner = {
path: string
usernames: string[]
}
export const getCodeOwnersInfo = (codeownerContent: string): CodeOwner[] => {
const lines = codeownerContent.split("\n")
const ownerEntries = [] as CodeOwner[]
for (const line of lines) {
if (!line) {
continue
}
if (line.startsWith("#")) {
continue
}
const [pathString, ...usernames] = line.split(/\s+/)
ownerEntries.push({
path: pathString,
usernames: usernames,
})
}
return ownerEntries
}

Просмотреть файл

@ -1,23 +0,0 @@
import { Octokit } from "@octokit/rest"
import { Logger } from "../util/logger"
type PullMeta = {
repo: string
owner: string
pull_number: number
}
export const mergeOrAddMergeLabel = async (api: Octokit, pullMeta: PullMeta, headCommitSHA: string, logger: Logger) => {
const allGreen = await api.repos.getCombinedStatusForRef({ ...pullMeta, ref: headCommitSHA })
if (allGreen.data.state === "success") {
logger.trace("Merging")
// Merge now
const commitTitle = "Merged automatically"
await api.pulls.merge({ ...pullMeta, commit_title: commitTitle })
} else {
logger.trace("Adding Merge on Green")
// Merge when green
await api.issues.addLabels({ ...pullMeta, labels: ["Merge on Green"], issue_number: pullMeta.pull_number })
}
}

Просмотреть файл

@ -1,8 +0,0 @@
import { Octokit, RestEndpointMethodTypes } from "@octokit/rest"
export const getContents = async (api: Octokit, opts: RestEndpointMethodTypes["repos"]["getContent"]["parameters"]) => {
const contentResponse = await api.repos.getContent(opts)
// @ts-ignore types are mismatched
const text = Buffer.from(contentResponse.data.content, "base64").toString()
return text
}

Просмотреть файл

@ -38,7 +38,6 @@ export const createMockGitHubClient = () => {
},
pulls: {
get: jest.fn(),
merge: jest.fn(),
listFiles: {
endpoint: {
merge: jest.fn(),
@ -96,7 +95,6 @@ export const createFakeGitHubClient = () => {
},
pulls: {
get: Promise.resolve({}),
merge: Promise.resolve({}),
listFiles: Promise.resolve({}),
},
checks: {