ci/github-script/merge: list eligible users in comment

When a user tries to merge a PR, but is not allowed to, it is helpful to
explicitly list the users who *are* allowed. This helps explaining *why*
the merge-eligible label was set.

I objected to this proposal before, because it would incur too many API
requests. But after we have restructured the checklist, this is not
actually true anymore - we can now sensibly run this only when a comment
is posted and not whenever we check a PR for eligibility.
This commit is contained in:
Wolfgang Walther 2025-11-04 11:21:14 +01:00
parent 58a1fe4761
commit 1e6124a504
No known key found for this signature in database
GPG key ID: B39893FA5F65CAE1
2 changed files with 34 additions and 1 deletions

View file

@ -119,6 +119,24 @@ module.exports = async ({ github, context, core, dry }) => {
return members[team_slug] return members[team_slug]
} }
// Caching users saves API requests when running the bot on the schedule and processing
// many PRs at once. It also helps to encapsulate the special logic we need, because
// actions/github doesn't support that endpoint fully, yet.
const users = {}
function getUser(id) {
if (!users[id]) {
users[id] = github
.request({
method: 'GET',
url: '/user/{id}',
id,
})
.then((resp) => resp.data)
}
return users[id]
}
async function handlePullRequest({ item, stats, events }) { async function handlePullRequest({ item, stats, events }) {
const log = (k, v) => core.info(`PR #${item.number} - ${k}: ${v}`) const log = (k, v) => core.info(`PR #${item.number} - ${k}: ${v}`)
@ -145,6 +163,7 @@ module.exports = async ({ github, context, core, dry }) => {
events, events,
maintainers, maintainers,
getTeamMembers, getTeamMembers,
getUser,
}) })
// When the same change has already been merged to the target branch, a PR will still be // When the same change has already been merged to the target branch, a PR will still be

View file

@ -80,6 +80,7 @@ function runChecklist({
return { return {
checklist, checklist,
eligible,
result, result,
} }
} }
@ -119,6 +120,7 @@ async function handleMerge({
events, events,
maintainers, maintainers,
getTeamMembers, getTeamMembers,
getUser,
}) { }) {
const pull_number = pull_request.number const pull_number = pull_request.number
@ -240,7 +242,7 @@ async function handleMerge({
} }
} }
const { result, checklist } = runChecklist({ const { result, eligible, checklist } = runChecklist({
committers, committers,
events, events,
files, files,
@ -270,6 +272,18 @@ async function handleMerge({
'', '',
] ]
if (eligible.size > 0 && !eligible.has(comment.user.id)) {
const users = await Promise.all(
Array.from(eligible, async (id) => (await getUser(id)).login),
)
body.push(
'> [!TIP]',
'> Maintainers eligible to merge are:',
...users.map((login) => `> - ${login}`),
'',
)
}
if (result) { if (result) {
await react('ROCKET') await react('ROCKET')
try { try {