ci/github-script/merge: move all conditions into runChecklist

No special casing anymore, all conditions are in the same place. This
also has the benefit of hiding the "has maintainers eligible for merge"
condition from comments, because it is only really relevant for
labeling.
This commit is contained in:
Wolfgang Walther 2025-11-02 10:36:52 +01:00
parent 7ea127c83a
commit 6a3c294f6f
No known key found for this signature in database
GPG key ID: B39893FA5F65CAE1

View file

@ -1,4 +1,12 @@
function runChecklist({ committers, files, pull_request, log, maintainers }) { function runChecklist({
committers,
files,
pull_request,
log,
maintainers,
user,
userIsMaintainer,
}) {
const packages = files const packages = files
.filter(({ filename }) => filename.startsWith('pkgs/by-name/')) .filter(({ filename }) => filename.startsWith('pkgs/by-name/'))
.map(({ filename }) => filename.split('/')[3]) .map(({ filename }) => filename.split('/')[3])
@ -21,7 +29,16 @@ function runChecklist({ committers, files, pull_request, log, maintainers }) {
'PR authored by r-ryantm or committer.': 'PR authored by r-ryantm or committer.':
pull_request.user.login === 'r-ryantm' || pull_request.user.login === 'r-ryantm' ||
committers.has(pull_request.user.id), committers.has(pull_request.user.id),
'PR has maintainers eligible for merge.': eligible.size > 0, }
if (user) {
checklist[`${user.login} can use the merge bot.`] = userIsMaintainer
checklist[
`${user.login} is eligible to merge changes to the touched packages.`
] = eligible.has(user.id)
} else {
// This is only used when no user is passed, i.e. for labeling.
checklist['PR has maintainers eligible to merge.'] = eligible.size > 0
} }
const result = Object.values(checklist).every(Boolean) const result = Object.values(checklist).every(Boolean)
@ -32,7 +49,6 @@ function runChecklist({ committers, files, pull_request, log, maintainers }) {
return { return {
checklist, checklist,
eligible,
result, result,
} }
} }
@ -107,14 +123,6 @@ async function handleMerge({
per_page: 100, per_page: 100,
}) })
const { checklist, eligible, result } = runChecklist({
committers,
files,
pull_request,
log,
maintainers,
})
// Only look through comments *after* the latest (force) push. // Only look through comments *after* the latest (force) push.
const latestChange = events.findLast(({ event }) => const latestChange = events.findLast(({ event }) =>
['committed', 'head_ref_force_pushed'].includes(event), ['committed', 'head_ref_force_pushed'].includes(event),
@ -224,9 +232,15 @@ async function handleMerge({
} }
} }
const canUseMergeBot = await isMaintainer(comment.user.login) const { result, checklist } = runChecklist({
const isEligible = eligible.has(comment.user.id) committers,
const canMerge = result && canUseMergeBot && isEligible files,
pull_request,
log,
maintainers,
user: comment.user,
userIsMaintainer: await isMaintainer(comment.user.login),
})
const body = [ const body = [
`<!-- comment: ${comment.node_id} -->`, `<!-- comment: ${comment.node_id} -->`,
@ -235,12 +249,10 @@ async function handleMerge({
...Object.entries(checklist).map( ...Object.entries(checklist).map(
([msg, res]) => `- :${res ? 'white_check_mark' : 'x'}: ${msg}`, ([msg, res]) => `- :${res ? 'white_check_mark' : 'x'}: ${msg}`,
), ),
`- :${canUseMergeBot ? 'white_check_mark' : 'x'}: ${comment.user.login} can use the merge bot.`,
`- :${isEligible ? 'white_check_mark' : 'x'}: ${comment.user.login} is eligible to merge changes to the touched packages.`,
'', '',
] ]
if (canMerge) { if (result) {
await react('ROCKET') await react('ROCKET')
try { try {
body.push(`:heavy_check_mark: ${await merge()} (#306934)`) body.push(`:heavy_check_mark: ${await merge()} (#306934)`)
@ -264,9 +276,17 @@ async function handleMerge({
}) })
} }
if (canMerge) break if (result) break
} }
const { result } = runChecklist({
committers,
files,
pull_request,
log,
maintainers,
})
// Returns a boolean, which indicates whether the PR is merge-bot eligible in principle. // Returns a boolean, which indicates whether the PR is merge-bot eligible in principle.
// This is used to set the respective label in bot.js. // This is used to set the respective label in bot.js.
return result return result