ci/github-script/bot: request reviewers
This migrates the bash code to request reviewers to github-script. This will allow multiple nice improvements later on, but at this stage it's mostly a reduction in code and complexity.
This commit is contained in:
@@ -5,6 +5,7 @@ module.exports = async ({ github, context, core, dry }) => {
|
||||
const withRateLimit = require('./withRateLimit.js')
|
||||
const { classify } = require('../supportedBranches.js')
|
||||
const { handleMerge } = require('./merge.js')
|
||||
const { handleReviewers } = require('./reviewers.js')
|
||||
|
||||
const artifactClient = new DefaultArtifactClient()
|
||||
|
||||
@@ -372,6 +373,30 @@ module.exports = async ({ github, context, core, dry }) => {
|
||||
maintainers[pkg]?.some((m) => approvals.has(m)),
|
||||
),
|
||||
})
|
||||
|
||||
if (!pull_request.draft) {
|
||||
await handleReviewers({
|
||||
github,
|
||||
context,
|
||||
core,
|
||||
log,
|
||||
dry,
|
||||
pull_request,
|
||||
reviews,
|
||||
// TODO: Use maintainer map instead of the artifact.
|
||||
maintainers: Object.keys(
|
||||
JSON.parse(
|
||||
await readFile(`${pull_number}/maintainers.json`, 'utf-8'),
|
||||
),
|
||||
).map((id) => parseInt(id)),
|
||||
// TODO: Create owner map similar to maintainer map.
|
||||
owners: (await readFile(`${pull_number}/owners.txt`, 'utf-8')).split(
|
||||
'\n',
|
||||
),
|
||||
getTeamMembers,
|
||||
getUser,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
return prLabels
|
||||
@@ -520,7 +545,7 @@ module.exports = async ({ github, context, core, dry }) => {
|
||||
const hasChanges = Object.keys(after).some(
|
||||
(name) => (before[name] ?? false) !== after[name],
|
||||
)
|
||||
if (log('Has changes', hasChanges, !hasChanges)) return
|
||||
if (log('Has label changes', hasChanges, !hasChanges)) return
|
||||
|
||||
// Skipping labeling on a pull_request event, because we have no privileges.
|
||||
const labels = Object.entries(after)
|
||||
|
||||
120
ci/github-script/reviewers.js
Normal file
120
ci/github-script/reviewers.js
Normal file
@@ -0,0 +1,120 @@
|
||||
async function handleReviewers({
|
||||
github,
|
||||
context,
|
||||
core,
|
||||
log,
|
||||
dry,
|
||||
pull_request,
|
||||
reviews,
|
||||
maintainers,
|
||||
owners,
|
||||
getTeamMembers,
|
||||
getUser,
|
||||
}) {
|
||||
const pull_number = pull_request.number
|
||||
|
||||
const users = new Set([
|
||||
...(await Promise.all(
|
||||
maintainers.map(async (id) => (await getUser(id)).login),
|
||||
)),
|
||||
...owners.filter((handle) => handle && !handle.includes('/')),
|
||||
])
|
||||
log('reviewers - users', Array.from(users).join(', '))
|
||||
|
||||
const teams = new Set(
|
||||
owners
|
||||
.map((handle) => handle.split('/'))
|
||||
.filter(([org, slug]) => org === context.repo.owner && slug)
|
||||
.map(([, slug]) => slug),
|
||||
)
|
||||
log('reviewers - teams', Array.from(teams).join(', '))
|
||||
|
||||
const team_members = new Set(
|
||||
(await Promise.all(Array.from(teams, getTeamMembers)))
|
||||
.flat(1)
|
||||
.map(({ login }) => login),
|
||||
)
|
||||
log('reviewers - team_members', Array.from(team_members).join(', '))
|
||||
|
||||
const new_reviewers = users
|
||||
.union(team_members)
|
||||
// We can't request a review from the author.
|
||||
.difference(new Set([pull_request.user?.login]))
|
||||
log('reviewers - new_reviewers', Array.from(new_reviewers).join(', '))
|
||||
|
||||
// Filter users to repository collaborators. If they're not, they can't be requested
|
||||
// for review. In that case, they probably missed their invite to the maintainers team.
|
||||
const reviewers = (
|
||||
await Promise.all(
|
||||
Array.from(new_reviewers, async (username) => {
|
||||
try {
|
||||
await github.rest.repos.checkCollaborator({
|
||||
...context.repo,
|
||||
username,
|
||||
})
|
||||
return username
|
||||
} catch (e) {
|
||||
if (e.status !== 404) throw e
|
||||
core.warn(
|
||||
`PR #${pull_number}: User ${username} cannot be requested for review because they don't exist or are not a repository collaborator, ignoring. They probably missed the automated invite to the maintainers team (see <https://github.com/NixOS/nixpkgs/issues/234293>).`,
|
||||
)
|
||||
}
|
||||
}),
|
||||
)
|
||||
).filter(Boolean)
|
||||
log('reviewers - reviewers', reviewers.join(', '))
|
||||
|
||||
if (reviewers.length > 15) {
|
||||
log(
|
||||
`Too many reviewers (${reviewers.join(', ')}), skipping review requests.`,
|
||||
)
|
||||
return
|
||||
}
|
||||
|
||||
const requested_reviewers = new Set(
|
||||
pull_request.requested_reviewers.map(({ login }) => login),
|
||||
)
|
||||
log(
|
||||
'reviewers - requested_reviewers',
|
||||
Array.from(requested_reviewers).join(', '),
|
||||
)
|
||||
|
||||
const existing_reviewers = new Set(
|
||||
reviews.map(({ user }) => user?.login).filter(Boolean),
|
||||
)
|
||||
log(
|
||||
'reviewers - existing_reviewers',
|
||||
Array.from(existing_reviewers).join(', '),
|
||||
)
|
||||
|
||||
const non_requested_reviewers = new Set(reviewers)
|
||||
.difference(requested_reviewers)
|
||||
// We don't want to rerequest reviews from people who already reviewed.
|
||||
.difference(existing_reviewers)
|
||||
log(
|
||||
'reviewers - non_requested_reviewers',
|
||||
Array.from(non_requested_reviewers).join(', '),
|
||||
)
|
||||
|
||||
if (non_requested_reviewers.size === 0) {
|
||||
log('Has reviewer changes', 'false (skipped)')
|
||||
} else if (dry) {
|
||||
core.info(
|
||||
`Requesting reviewers for #${pull_number}: ${Array.from(non_requested_reviewers).join(', ')} (dry)`,
|
||||
)
|
||||
} else {
|
||||
// We had tried the "request all reviewers at once" thing in the past, but it didn't work out:
|
||||
// https://github.com/NixOS/nixpkgs/commit/034613f860fcd339bd2c20c8f6bc259a2f9dc034
|
||||
// If we're hitting API errors here again, we'll need to investigate - and possibly reverse
|
||||
// course.
|
||||
await github.rest.pulls.requestReviewers({
|
||||
...context.repo,
|
||||
pull_number,
|
||||
reviewers,
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
module.exports = {
|
||||
handleReviewers,
|
||||
}
|
||||
Reference in New Issue
Block a user