diff --git a/.github/workflows/approve-contributor.yml b/.github/workflows/approve-contributor.yml index d2d8a58d770..409a4c32e67 100644 --- a/.github/workflows/approve-contributor.yml +++ b/.github/workflows/approve-contributor.yml @@ -6,11 +6,11 @@ on: jobs: approve: - if: ${{ !github.event.issue.pull_request }} runs-on: ubuntu-latest permissions: contents: write issues: write + pull-requests: write steps: - name: Checkout uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 @@ -30,17 +30,18 @@ jobs: const commenter = context.payload.comment.user.login; const commentBody = (context.payload.comment.body || '').trim(); - let targetCapability; - if (/\blgtmi\b/i.test(commentBody)) { - targetCapability = 'issue'; - } else if (/\blgtm\b/i.test(commentBody)) { - targetCapability = 'pr'; - } else { - console.log('Comment does not match lgtm or lgtmi'); + const approvalAtStartPattern = /^[\s.]*(?:@[A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?(?:\s*,\s*|[.:]\s*|\s+))*(lgtmi|lgtm)(?=$|[\s.])/i; + const approvalAtEndPattern = /(?:^|[\s.])(lgtmi|lgtm)[\s.]*$/i; + const approvalMatch = commentBody.match(approvalAtStartPattern) ?? commentBody.match(approvalAtEndPattern); + + if (!approvalMatch) { + console.log('Comment does not start or end with lgtm or lgtmi'); core.setOutput('status', 'skipped'); return; } + const targetCapability = approvalMatch[1].toLowerCase() === 'lgtmi' ? 'issue' : 'pr'; + try { const { data: permissionLevel } = await github.rest.repos.getCollaboratorPermissionLevel({ owner: context.repo.owner, @@ -59,6 +60,24 @@ jobs: return; } + function parseMentionedUsers(body) { + const users = []; + const seenUsers = new Set(); + const mentionPattern = /(^|[^A-Za-z0-9_])@([A-Za-z0-9](?:[A-Za-z0-9-]{0,37}[A-Za-z0-9])?)(?![A-Za-z0-9-]|\/)/g; + + for (const match of body.matchAll(mentionPattern)) { + const username = match[2]; + const normalizedUser = username.toLowerCase(); + if (seenUsers.has(normalizedUser)) { + continue; + } + seenUsers.add(normalizedUser); + users.push(username); + } + + return users; + } + function parseApprovedUsers(content) { const lines = content.split('\n'); const entries = []; @@ -113,63 +132,88 @@ jobs: const content = fs.readFileSync(APPROVED_FILE, 'utf8'); const { entries, users } = parseApprovedUsers(content); - const normalizedAuthor = issueAuthor.toLowerCase(); - const existingEntry = users.get(normalizedAuthor); - const existingCapability = existingEntry?.capability ?? null; + const mentionedUsers = parseMentionedUsers(commentBody); + const approvalTargets = mentionedUsers.length > 0 ? mentionedUsers : [issueAuthor]; + const changedTargets = []; + const alreadyTargets = []; + + for (const username of approvalTargets) { + const normalizedUser = username.toLowerCase(); + const existingEntry = users.get(normalizedUser); + const existingCapability = existingEntry?.capability ?? null; + + if (existingCapability === 'pr' || existingCapability === targetCapability) { + alreadyTargets.push(existingEntry?.username ?? username); + console.log(`${username} is already approved for ${existingCapability}`); + continue; + } - if (existingCapability === 'pr' || existingCapability === targetCapability) { - core.setOutput('status', 'already'); - core.setOutput('capability', existingCapability); - console.log(`${issueAuthor} is already approved for ${existingCapability}`); - return; + if (existingEntry) { + existingEntry.capability = targetCapability; + changedTargets.push(existingEntry.username); + } else { + const entry = { type: 'user', username, normalizedUser, capability: targetCapability }; + entries.push(entry); + users.set(normalizedUser, entry); + changedTargets.push(username); + } + + console.log(`Set ${username} capability to ${targetCapability}`); } - if (existingEntry) { - existingEntry.capability = targetCapability; - } else { - entries.push({ type: 'user', username: issueAuthor, normalizedUser: normalizedAuthor, capability: targetCapability }); + core.setOutput('capability', targetCapability); + core.setOutput('changed_targets', JSON.stringify(changedTargets)); + core.setOutput('already_targets', JSON.stringify(alreadyTargets)); + + if (changedTargets.length === 0) { + core.setOutput('status', 'already'); + return; } fs.writeFileSync(APPROVED_FILE, stringifyApprovedUsers(entries)); - core.setOutput('status', existingCapability ? 'updated' : 'added'); - core.setOutput('capability', targetCapability); - console.log(`Set ${issueAuthor} capability to ${targetCapability}`); + core.setOutput('status', 'changed'); - name: Commit and push - if: steps.update.outputs.status == 'added' || steps.update.outputs.status == 'updated' + if: steps.update.outputs.status == 'changed' run: | git config user.name "github-actions[bot]" git config user.email "github-actions[bot]@users.noreply.github.com" git add .github/APPROVED_CONTRIBUTORS - git diff --staged --quiet || git commit -m "chore: approve contributor ${{ github.event.issue.user.login }}" + git diff --staged --quiet || git commit -m "chore: approve contributors from issue #${{ github.event.issue.number }}" git push - name: Comment on issue - if: steps.update.outputs.status == 'added' || steps.update.outputs.status == 'updated' || steps.update.outputs.status == 'already' + if: steps.update.outputs.status == 'changed' || steps.update.outputs.status == 'already' uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0 + env: + CAPABILITY: ${{ steps.update.outputs.capability }} + CHANGED_TARGETS: ${{ steps.update.outputs.changed_targets }} + ALREADY_TARGETS: ${{ steps.update.outputs.already_targets }} with: script: | - const issueAuthor = context.payload.issue.user.login; - const capability = '${{ steps.update.outputs.capability }}'; + const capability = process.env.CAPABILITY; + const changedTargets = JSON.parse(process.env.CHANGED_TARGETS || '[]'); + const alreadyTargets = JSON.parse(process.env.ALREADY_TARGETS || '[]'); const defaultBranch = context.payload.repository.default_branch; - let body; - - if ('${{ steps.update.outputs.status }}' === 'already') { - body = `@${issueAuthor} is already approved.`; - } else if (capability === 'issue') { - body = [ - `@${issueAuthor} approved for issues. Your future issues will not be auto-closed. PRs still require \`lgtm\`.`, - '', - `See [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md).`, - ].join('\n'); - } else { - body = [ - `@${issueAuthor} approved for issues and PRs. Your future issues and PRs will not be auto-closed.`, - '', - `See [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md).`, - ].join('\n'); + const formatTargets = (targets) => targets.map((target) => `@${target}`).join(', '); + const bodyLines = []; + + if (changedTargets.length > 0) { + if (capability === 'issue') { + bodyLines.push(`${formatTargets(changedTargets)} approved for issues. Future issues will not be auto-closed. PRs still require \`lgtm\` at the start of a maintainer reply (optionally after one or more \`@username\` mentions) or at the end.`); + } else { + bodyLines.push(`${formatTargets(changedTargets)} approved for issues and PRs. Future issues and PRs will not be auto-closed.`); + } } + if (alreadyTargets.length > 0) { + const verb = alreadyTargets.length === 1 ? 'is' : 'are'; + bodyLines.push(`${formatTargets(alreadyTargets)} ${verb} already approved.`); + } + + bodyLines.push('', `See [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md).`); + const body = bodyLines.join('\n'); + await github.rest.issues.createComment({ owner: context.repo.owner, repo: context.repo.repo, diff --git a/.github/workflows/issue-gate.yml b/.github/workflows/issue-gate.yml index 02d4000d1a7..03372acaead 100644 --- a/.github/workflows/issue-gate.yml +++ b/.github/workflows/issue-gate.yml @@ -101,7 +101,7 @@ jobs: '', `Maintainers review auto-closed issues daily and reopen worthwhile ones. Issues that do not meet the quality bar in [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md) will not be reopened or receive a reply.`, '', - 'If a maintainer replies `lgtmi` on one of your issues, your future issues will stay open. If a maintainer replies `lgtm`, your future issues and PRs will stay open.', + 'If a maintainer replies `lgtmi` on one of your issues, your future issues will stay open. If a maintainer replies `lgtm`, your future issues and PRs will stay open. The command must be at the start of the reply (optionally after one or more `@username` mentions) or at the end.', '', `See [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md).`, ].join('\n'); diff --git a/.github/workflows/pr-gate.yml b/.github/workflows/pr-gate.yml index 48e0745fb0d..bc6f7eef11d 100644 --- a/.github/workflows/pr-gate.yml +++ b/.github/workflows/pr-gate.yml @@ -116,11 +116,11 @@ jobs: console.log(`${prAuthor} is not approved, closing PR`); const message = [ - 'This PR was auto-closed. Only contributors approved with `lgtm` can open PRs. Open an issue first.', + 'This PR was auto-closed. Only contributors approved with `lgtm` can open PRs. Open an issue first and ask a maintainer for approval.', '', `Maintainers review auto-closed issues daily. Issues that do not meet the quality bar in [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md) will not be reopened or receive a reply.`, '', - 'If a maintainer replies `lgtmi`, your future issues will stay open. If a maintainer replies `lgtm`, your future issues and PRs will stay open.', + 'If a maintainer replies `lgtmi`, your future issues will stay open. If a maintainer replies `lgtm`, your future issues and PRs will stay open. The command must be at the start of the reply (optionally after one or more `@username` mentions) or at the end.', '', `See [CONTRIBUTING.md](https://github.com/${context.repo.owner}/${context.repo.repo}/blob/${defaultBranch}/CONTRIBUTING.md).`, ].join('\n'); diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index b0c38322dc0..c6d76f1c574 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -31,7 +31,7 @@ Approval happens through maintainer replies on issues: - `lgtmi`: your future issues will not be auto-closed - `lgtm`: your future issues and PRs will not be auto-closed -`lgtmi` does not grant rights to submit PRs. Only `lgtm` grants rights to submit PRs. +The command must be at the start of the reply (optionally after one or more `@username` mentions) or at the end. `lgtmi` does not grant rights to submit PRs. Only `lgtm` grants rights to submit PRs. ## Quality Bar For Issues @@ -45,7 +45,7 @@ If you open an issue, keep it short, concrete, and worth reading. - Explain why it matters. - If you want to implement the change yourself, say so. -If the issue is real and written well, a maintainer may reopen it, reply `lgtmi`, or reply `lgtm`. +If the issue is real and written well, a maintainer may reopen it or reply with `lgtmi` or `lgtm` in the command position described above. ## Blocking @@ -55,7 +55,7 @@ If you send a large volume of issues through automation, your GitHub account wil ## Before Submitting a PR -Do not open a PR unless you have already been approved with `lgtm`. +Do not open a PR unless you have already been approved by a maintainer using `lgtm` in the command position described above. Before submitting a PR: