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

feat: ping reviewers based on which files were changed #265

Merged
merged 1 commit into from
Aug 7, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion lib/logger.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const path = require('path')
const bunyan = require('bunyan')

const isRunningTests = process.env.npm_lifecycle_event === 'test'
const stdoutLevel = isRunningTests ? 'FATAL' : 'INFO'
const stdoutLevel = isRunningTests ? 'FATAL' : process.env.LOG_LEVEL || 'INFO'

const daysToKeepLogs = process.env.KEEP_LOGS || 10
const logsDir = process.env.LOGS_DIR || ''
Expand Down
31 changes: 31 additions & 0 deletions lib/node-owners.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
'use static'

const { parse } = require('codeowners-utils')
// NOTE(mmarchini): `codeowners-utils` doesn't respect ./ prefix,
// so we use micromatch
const micromatch = require('micromatch')

class Owners {
constructor (ownersDefinitions) {
this._ownersDefinitions = ownersDefinitions
}

static fromFile (content) {
return new Owners(parse(content))
}

getOwnersForPaths (paths) {
let ownersForPath = []
for (const { pattern, owners } of this._ownersDefinitions) {
if (micromatch(paths, pattern).length > 0) {
ownersForPath = ownersForPath.concat(owners)
}
}
// Remove duplicates before returning
return ownersForPath.filter((v, i) => ownersForPath.indexOf(v) === i).sort()
}
}

module.exports = {
Owners
}
109 changes: 109 additions & 0 deletions lib/node-repo.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,13 @@

const LRU = require('lru-cache')
const retry = require('async').retry
const Aigle = require('aigle')
const request = require('request')

const githubClient = require('./github-client')
const { createPrComment } = require('./github-comment')
const resolveLabels = require('./node-labels').resolveLabels
const { Owners } = require('./node-owners')
const existingLabelsCache = new LRU({ max: 1, maxAge: 1000 * 60 * 60 })

const fiveSeconds = 5 * 1000
Expand Down Expand Up @@ -185,10 +189,115 @@ function stringsInCommon (arr1, arr2) {
return arr1.filter((str) => loweredArr2.indexOf(str.toLowerCase()) !== -1)
}

async function deferredResolveOwnersThenPingPr (options) {
const timeoutMillis = (options.timeoutInSec || 0) * 1000
await sleep(timeoutMillis)
return resolveOwnersThenPingPr(options)
}

function getCodeOwnersUrl (owner, repo, defaultBranch) {
const base = 'raw.githubusercontent.com'
const filepath = '.github/CODEOWNERS'
return `https://${base}/${owner}/${repo}/${defaultBranch}/${filepath}`
}

async function getFiles ({ owner, repo, number, logger }) {
try {
const response = await githubClient.pullRequests.getFiles({
owner,
repo,
number
})
return response.data.map(({ filename }) => filename)
} catch (err) {
logger.error(err, 'Error retrieving files from GitHub')
throw err
}
}

async function getDefaultBranch ({ owner, repo, logger }) {
try {
const data = (await githubClient.repos.get({ owner, repo })).data || { }

if (!data['default_branch']) {
logger.error(null, 'Could not determine default branch')
throw new Error('unknown default branch')
}

return data.default_branch
} catch (err) {
logger.error(err, 'Error retrieving repository data')
throw err
}
}

function getCodeOwnersFile (url, { logger }) {
return new Promise((resolve, reject) => {
request(url, (err, res, body) => {
if (err || res.statusCode !== 200) {
logger.error(err, 'Error retrieving OWNERS')
return reject(err)
}
return resolve(body)
})
})
}

async function resolveOwnersThenPingPr (options) {
const { owner, repo } = options
const times = options.retries || 5
const interval = options.retryInterval || fiveSeconds
const retry = fn => Aigle.retry({ times, interval }, fn)

options.logger.debug('Getting file paths')
options.number = options.prId
const filepathsChanged = await retry(() => getFiles(options))

options.logger.debug('Getting default branch')
const defaultBranch = await retry(() => getDefaultBranch(options))

const url = getCodeOwnersUrl(owner, repo, defaultBranch)
options.logger.debug(`Fetching OWNERS on ${url}`)

const file = await retry(() => getCodeOwnersFile(url, options))

options.logger.debug('Parsing codeowners file')
const owners = Owners.fromFile(file)
const selectedOwners = owners.getOwnersForPaths(filepathsChanged)

options.logger.debug('Pinging codeowners file')
if (selectedOwners.length > 0) {
await pingOwners(options, selectedOwners)
}
}

function getCommentForOwners (owners) {
return `Review requested:\n\n${owners.map(i => `- [ ] ${i}`).join('\n')}`
}

async function pingOwners (options, owners) {
try {
await createPrComment({
owner: options.owner,
repo: options.repo,
number: options.prId,
logger: options.logger
}, getCommentForOwners(owners))
} catch (err) {
options.logger.error(err, 'Error while pinging owners')
throw err
}
options.logger.debug('Pinged owners: ' + owners)
}

exports.getBotPrLabels = getBotPrLabels
exports.removeLabelFromPR = removeLabelFromPR
exports.fetchExistingThenUpdatePr = fetchExistingThenUpdatePr
exports.resolveLabelsThenUpdatePr = deferredResolveLabelsThenUpdatePr
exports.resolveOwnersThenPingPr = deferredResolveOwnersThenPingPr

// exposed for testability
exports._fetchExistingLabels = fetchExistingLabels
exports._testExports = {
pingOwners, getCodeOwnersFile, getCodeOwnersUrl, getDefaultBranch, getFiles, getCommentForOwners
}