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

Automatically assign PR reviewers #4

Merged
merged 13 commits into from
Aug 7, 2020
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,7 @@ node_modules
# tests
coverage
npm-debug.log

# editors
.vscode
.idea
122 changes: 122 additions & 0 deletions src/actions/assign.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { Context, Octokit } from "probot"
import { different } from "../utils"

export default async function assignPullRequestReviewers(context: Context) {
const PRnumber = context.payload.number
const owner = context.payload.repository.owner.login
const repo = context.payload.repository.name

context.log.info("PR number:", PRnumber, ", owner:", owner, ", repo:", repo)

const oldReviewers: string[] = context.payload.pull_request.requested_reviewers.map(
(reviewer: { login: string }) => reviewer.login
)

const newReviewers = await reviewersOfPR(context, PRnumber, owner, repo)

// Check if old and new reviewers differ. Otherwise we can skip an API call
if (different(oldReviewers, newReviewers)) {
// Log reviewers replacement
context.log.info("Old reviewers:", oldReviewers)
context.log.info("New reviewers:", newReviewers)

const params = context.repo({ pull_number: PRnumber, reviewers: newReviewers })
const response = await context.github.pulls.createReviewRequest(params)
context.log.info("Status:", response.headers.status)
} else {
context.log.info("No reviewers changed")
}
}

interface CodeOwner {
path: string
username: string
}

function parseModifiedFiles(filesResponse: Octokit.PullsListFilesResponse): string[] {
const modifiedFiles: string[] = []

for (const file of filesResponse) {
modifiedFiles.push(file.filename)
}

return modifiedFiles
}

function parseCodeOwners(codeOwnersFile: string): CodeOwner[] {
const codeOwners: CodeOwner[] = []
for (const line of codeOwnersFile.split("\n")) {
// Ignore empty lines and comments
if (line.length === 0 || line.startsWith("#")) {
bartekpacia marked this conversation as resolved.
Show resolved Hide resolved
continue
}

let [path, owner] = line.split(" ")

// Allow only owners in the form of @username
if (!owner?.startsWith("@")) {
continue
}

const codeOwner: CodeOwner = {
path: path,
username: owner.slice(1) // Remove initial '@'
}

codeOwners.push(codeOwner)
}

return codeOwners
}

/**
* @return List of maintainers of the files that were modified in `PRnumber`
*/
async function reviewersOfPR(
context: Context,
PRnumber: number,
repoOwner: string,
repo: string
): Promise<string[]> {
const reviewers: string[] = []

// Get list of modified files
const modifiedFilesResponse = await context.github.pulls.listFiles({
owner: repoOwner,
repo: repo,
pull_number: PRnumber,
})

const filesData: any = modifiedFilesResponse.data
const modifiedFiles = parseModifiedFiles(filesData)

// Get CODEOWNERS file contents
const codeOwnersResponse = await context.github.repos.getContents({
owner: repoOwner,
repo: repo,
path: ".github/CODEOWNERS",
})

const codeOwnersData: any = codeOwnersResponse.data
const codeOwnersFile = Buffer.from(codeOwnersData.content, "base64").toString()
const codeOwners = parseCodeOwners(codeOwnersFile)

// Assumptions:
// - There can be many files owned by the same owner
// - There can be many owners for the same file. For example:
// /plugins/git/git.plugin.zsh could have 2 owners (for /plugins and for /plugins/git)
// - The number of codeOwners >> the number of modified files
for (const codeOwner of codeOwners) {
for (const path of modifiedFiles) {
if (path.includes(codeOwner.path)) {
context.log.info(`${path} was modified. ${codeOwner.username} owns ${codeOwner.path}`)
reviewers.push(codeOwner.username)
break // This user is a reviewer, let's look at the next one
}
}
}

return reviewers
}

export { parseModifiedFiles, parseCodeOwners, CodeOwner }
23 changes: 8 additions & 15 deletions src/actions/triage.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import { Context } from 'probot' // eslint-disable-line no-unused-vars
import { execSync } from 'child_process'
import { different } from "../utils"

// List of labels determined *only* via the triage function.
// The result of the triage function will have precedence
// on these labels over a maintainer having applied them from
// the GitHub web UI.
import codeLabels from './labels.json'

export = async function triage (context: Context) {
export default async function triage (context: Context) {
// Get PR number and current PR labels
const PRNumber = context.payload.number
const oldLabels: string[] = context.payload.pull_request.labels.map((label: { name: string }) => label.name)
Expand All @@ -21,9 +22,10 @@ export = async function triage (context: Context) {
process.env['REPO_URL'] = repoURL
process.env['REPO_DIR'] = `${process.cwd()}/github`

let newLabels: string[] = []
try {
// Get new PR labels
var newLabels = labelsOfPR(PRNumber)
newLabels = labelsOfPR(PRNumber)
} catch (err) {
context.log.error(err, 'Error in triage.zsh script call')
return
Expand All @@ -45,6 +47,10 @@ export = async function triage (context: Context) {
}
}

/**
* Executes the zsh script that assign labels to the PR of PRNumber.
* The zsh script assign the labels basing on the files that have been changed.
*/
function labelsOfPR (PRNumber: number): string[] {
// Path to script based on app root
const zshScript = `${process.cwd()}/src/actions/triage.zsh`
Expand All @@ -57,16 +63,3 @@ function labelsOfPR (PRNumber: number): string[] {
// NOTE: this assumes there are no "s in the original string
return JSON.parse(stdout.replace(/'/g, '"'))
}

function different (A: string[], B: string[]): boolean {
if (A.length !== B.length) return true

A.sort()
B.sort()

for (let i = 0; i < A.length; i++) {
if (A[i] !== B[i]) return true
}

return false
}
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import { Application } from 'probot' // eslint-disable-line no-unused-vars
import triagePullRequest from './actions/triage'
import assignPullRequestReviewers from './actions/assign'

export = (app: Application) => {
app.on(['pull_request.opened', 'pull_request.synchronize'], triagePullRequest)
app.on(['pull_request.opened', 'pull_request.synchronize'], assignPullRequestReviewers)
}
14 changes: 14 additions & 0 deletions src/utils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
function different (A: string[], B: string[]): boolean {
if (A.length !== B.length) return true

A.sort()
B.sort()

for (let i = 0; i < A.length; i++) {
if (A[i] !== B[i]) return true
}

return false
}

export { different }
30 changes: 30 additions & 0 deletions test/assign.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { promises as fs } from 'fs'
import { parseModifiedFiles, parseCodeOwners, CodeOwner } from '../src/actions/assign' // eslint-disable-line no-unused-vars
import modifiedFilesResponse from '../test/fixtures/modifiedFiles.json'

describe('assigning PR reviewers', () => {
test('correctly parses modified files', async () => {
const expectedModifiedFiles = ['plugins/gitfast/update']
const modifiedFiles = await parseModifiedFiles(modifiedFilesResponse)

expect(modifiedFiles.sort()).toEqual(expectedModifiedFiles.sort())
})

test('correctly parses CODEOWNERS file', async () => {
const expectedCodeOwners: CodeOwner[] = [
{
path: 'plugins/gitfast/',
username: 'felipec'
},
{
path: 'plugins/sdk/',
username: 'rgoldberg'
}
]

const codeOwnersFile = await fs.readFile('./test/fixtures/CODEOWNERS')
const codeOwners = await parseCodeOwners(codeOwnersFile.toString())

expect(codeOwners.sort()).toEqual(expectedCodeOwners.sort())
})
})
3 changes: 3 additions & 0 deletions test/fixtures/CODEOWNERS
Validating CODEOWNERS rules …
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
# Plugin owners
plugins/gitfast/ @felipec
plugins/sdk/ @rgoldberg
15 changes: 0 additions & 15 deletions test/fixtures/issues.opened.json

This file was deleted.

14 changes: 14 additions & 0 deletions test/fixtures/modifiedFiles.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
"sha": "40050a704603103d24d37a777cff345a530f34d9",
"filename": "plugins/gitfast/update",
"status": "modified",
"additions": 2,
"deletions": 0,
"changes": 2,
"blob_url": "https://github.com/bartekpacia/ohmyzsh/blob/5383477f0701be7802f356fbc5c5db118dc4edd8/plugins/gitfast/update",
"raw_url": "https://github.com/bartekpacia/ohmyzsh/raw/5383477f0701be7802f356fbc5c5db118dc4edd8/plugins/gitfast/update",
"contents_url": "https://api.github.com/repos/bartekpacia/ohmyzsh/contents/plugins/gitfast/update?ref=5383477f0701be7802f356fbc5c5db118dc4edd8",
"patch": "@@ -7,3 +7,5 @@ curl -s -o _git \"${url}/git-completion.zsh?h=v${version}\" &&\n curl -s -o git-completion.bash \"${url}/git-completion.bash?h=v${version}\" &&\n curl -s -o git-prompt.sh \"${url}/git-prompt.sh?h=v${version}\" &&\n git apply updates.patch\n+\n+echo \"This is an example change to a plugin!\"\n\\ No newline at end of file"
}
]
Loading