Skip to content

Commit 4f98409

Browse files
authored
Disable @-mentions in approval workflow comments (#4965)
## Why The approval workflow comments currently @-mention everyone it suggests as a reviewer. This generates a lot of notification noise, especially on PRs that touch multiple ownership areas. The comments are useful for showing who should review, but the pings are disruptive. ## Changes Before: approval comments used `@username` for all suggested reviewers, eligible owners, and maintainers, triggering GitHub notifications for each. Now: all mentions are wrapped in backticks (`` `@username` ``), so they render as inline code on GitHub. This preserves the familiar `@` prefix for readability but prevents GitHub from treating them as mentions that trigger notifications. All hardcoded `@` mentions in the comment templates are now routed through a single `fmtLogin` helper controlled by the existing `MENTION_REVIEWERS` flag. ### Example comment (cross-domain PR) > ## Approval status: pending > > ### `/cmd/pipelines/` - approved by `@jefferycheng1` > Files: `cmd/pipelines/foo.go` > > ### `/bundle/` - needs approval > Files: `bundle/config.go` > Eligible: `@bundleowner1`, `@bundleowner2`, `@bundleowner3` > > <sub>Any maintainer (`@maintainer1`, `@maintainer2`, `@maintainer3`) can approve all areas. > See OWNERS for ownership rules.</sub> ## Test plan - All 20 existing unit tests pass - Ran a verification script against 5 scenarios (single domain, cross-domain partial approval, wildcard-only, team-owned paths, three domains mixed) confirming zero bare @-mentions in generated comments
1 parent 881c540 commit 4f98409

2 files changed

Lines changed: 14 additions & 11 deletions

File tree

.github/workflows/maintainer-approval.js

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ async function checkPerPathApproval(files, rulesWithTeams, approverLogins, githu
9696

9797
// --- Git history & scoring helpers ---
9898

99-
const MENTION_REVIEWERS = true;
99+
const MENTION_REVIEWERS = false;
100100
const OWNERS_LINK = "[OWNERS](.github/OWNERS)";
101101
const MARKER = "<!-- MAINTAINER_APPROVAL -->";
102102
const STATUS_CONTEXT = "maintainer-approval";
@@ -203,9 +203,8 @@ function topDirs(ds, n = 3) {
203203
}
204204

205205
function fmtReviewer(login, dirs) {
206-
const mention = MENTION_REVIEWERS ? `@${login}` : login;
207206
const dirList = dirs.map((d) => `\`${d}/\``).join(", ");
208-
return `- ${mention} -- recent work in ${dirList}`;
207+
return `- ${fmtLogin(login)} -- recent work in ${dirList}`;
209208
}
210209

211210
function selectReviewers(ss) {
@@ -221,8 +220,12 @@ function selectReviewers(ss) {
221220
}
222221

223222
function fmtEligible(owners) {
224-
if (MENTION_REVIEWERS) return owners.map((o) => `@${o}`).join(", ");
225-
return owners.join(", ");
223+
return owners.map((o) => fmtLogin(o)).join(", ");
224+
}
225+
226+
function fmtLogin(login) {
227+
if (MENTION_REVIEWERS) return `@${login}`;
228+
return `\`@${login}\``;
226229
}
227230

228231
async function countRecentReviews(github, owner, repo, logins, days = 30) {
@@ -267,7 +270,7 @@ function buildPendingPerGroupComment(groups, scores, dirScores, approvedBy, main
267270

268271
const approver = approvedBy.get(pattern);
269272
if (approver) {
270-
lines.push(`### \`${pattern}\` - approved by @${approver}`);
273+
lines.push(`### \`${pattern}\` - approved by ${fmtLogin(approver)}`);
271274
} else {
272275
lines.push(`### \`${pattern}\` - needs approval`);
273276
}
@@ -277,13 +280,13 @@ function buildPendingPerGroupComment(groups, scores, dirScores, approvedBy, main
277280
const individuals = owners.filter(o => !o.includes("/") && o.toLowerCase() !== authorLower);
278281

279282
if (teams.length > 0) {
280-
lines.push(`Teams: ${teams.map(t => `@${t}`).join(", ")}`);
283+
lines.push(`Teams: ${teams.map(t => fmtLogin(t)).join(", ")}`);
281284
}
282285

283286
if (!approver && individuals.length > 0) {
284287
const scored = individuals.map(o => [o, scores[o] || 0]).sort((a, b) => b[1] - a[1]);
285288
if (scored[0][1] > 0) {
286-
lines.push(`Suggested: @${scored[0][0]}`);
289+
lines.push(`Suggested: ${fmtLogin(scored[0][0])}`);
287290
const rest = scored.slice(1).map(([o]) => o);
288291
if (rest.length > 0) {
289292
lines.push(`Also eligible: ${fmtEligible(rest)}`);
@@ -320,7 +323,7 @@ function buildPendingPerGroupComment(groups, scores, dirScores, approvedBy, main
320323

321324
const maintainerList = maintainers
322325
.filter(m => m.toLowerCase() !== authorLower)
323-
.map(m => `@${m}`)
326+
.map(m => fmtLogin(m))
324327
.join(", ");
325328

326329
lines.push(
@@ -349,7 +352,7 @@ function buildSingleDomainPendingComment(sortedScores, dirScores, scoredCount, e
349352
} else if (roundRobinReviewer) {
350353
lines.push(
351354
"Could not determine reviewers from git history.",
352-
`Round-robin suggestion: @${roundRobinReviewer}`,
355+
`Round-robin suggestion: ${fmtLogin(roundRobinReviewer)}`,
353356
""
354357
);
355358
}

.github/workflows/maintainer-approval.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -509,7 +509,7 @@ describe("maintainer-approval", () => {
509509
assert.ok(body.includes("## Approval status: pending"));
510510
assert.ok(body.includes("`/cmd/pipelines/`"));
511511
assert.ok(body.includes("`/bundle/`"));
512-
assert.ok(body.includes("approved by @jefferycheng1"));
512+
assert.ok(body.includes("approved by `@jefferycheng1`"));
513513
assert.ok(body.includes("needs approval"));
514514
});
515515
});

0 commit comments

Comments
 (0)