Skip to content
This repository was archived by the owner on Jul 1, 2024. It is now read-only.

Commit ed742ca

Browse files
jablkojablko
committed
Fix empty owners-to-ping in staleness comments
1 parent 5485345 commit ed742ca

File tree

2 files changed

+44
-47
lines changed

2 files changed

+44
-47
lines changed

‎src/comments.ts‎

Lines changed: 36 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -150,42 +150,39 @@ export const StalenessExplanations: { [k: string]: string } = {
150150
};
151151

152152
// Comments to post for the staleness timeline (the tag is computed in `makeStaleness`)
153-
export const StalenessComment = (author: string, ownersToPing: string[], expires: string) => {
154-
const ownerPing = ownersToPing.map(o => "@"+o).join(", ");
155-
return {
156-
// --Unmerged--
157-
"Unmerged:nearly": txt`
158-
|Re-ping @${author} / ${ownerPing}:
159-
|
160-
|This PR has been ready to merge for over a week, and I haven't seen any requests to
161-
merge it. I will close it on ${expires} (in three weeks) if this doesn't happen.
162-
|
163-
|(If there's no reason to avoid merging it, please do so. Otherwise, if it shouldn't
164-
be merged or if it needs more time, please close it or turn it into a draft.)`,
165-
"Unmerged:done": txt`
166-
|After a month, no one has requested merging the PR 😞. I'm going to assume that the
167-
change is not wanted after all, and will therefore close it.`,
168-
// --Abandoned--
169-
"Abandoned:nearly": txt`
170-
|@${author} I haven't seen any activity on this PR in more than three weeks, and it
171-
still has problems that prevent it from being merged. The PR will be closed on
172-
${expires} (in a week) if the issues aren't addressed.`,
173-
"Abandoned:done": txt`
174-
|@${author} To keep things tidy, we have to close PRs that aren't mergeable and don't
175-
have activity in the last month. No worries, though — please open a new PR if you'd
176-
like to continue with this change. Thank you!`,
177-
// --Unreviewed--
178-
"Unreviewed:nearly": txt`
179-
|Re-ping ${ownerPing}:
180-
|
181-
|This PR has been out for over a week, yet I haven't seen any reviews.
182-
|
183-
|Could someone please give it some attention? Thanks!`,
184-
"Unreviewed:done": txt`
185-
|It has been more than two weeks and this PR still has no reviews.
186-
|
187-
|I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}.
188-
|
189-
|(Ping ${ownerPing}.)`
190-
} as { [k: string]: string };
191-
};
153+
export const StalenessComment = (author: string, ownersToPing: string, expires: string): { [k: string]: string } => ({
154+
// --Unmerged--
155+
"Unmerged:nearly": txt`
156+
|Re-ping @${author} / ${ownersToPing}:
157+
|
158+
|This PR has been ready to merge for over a week, and I haven't seen any requests to
159+
merge it. I will close it on ${expires} (in three weeks) if this doesn't happen.
160+
|
161+
|(If there's no reason to avoid merging it, please do so. Otherwise, if it shouldn't
162+
be merged or if it needs more time, please close it or turn it into a draft.)`,
163+
"Unmerged:done": txt`
164+
|After a month, no one has requested merging the PR 😞. I'm going to assume that the
165+
change is not wanted after all, and will therefore close it.`,
166+
// --Abandoned--
167+
"Abandoned:nearly": txt`
168+
|@${author} I haven't seen any activity on this PR in more than three weeks, and it
169+
still has problems that prevent it from being merged. The PR will be closed on
170+
${expires} (in a week) if the issues aren't addressed.`,
171+
"Abandoned:done": txt`
172+
|@${author} To keep things tidy, we have to close PRs that aren't mergeable and don't
173+
have activity in the last month. No worries, though — please open a new PR if you'd
174+
like to continue with this change. Thank you!`,
175+
// --Unreviewed--
176+
"Unreviewed:nearly": txt`
177+
|Re-ping ${ownersToPing}:
178+
|
179+
|This PR has been out for over a week, yet I haven't seen any reviews.
180+
|
181+
|Could someone please give it some attention? Thanks!`,
182+
"Unreviewed:done": txt`
183+
|It has been more than two weeks and this PR still has no reviews.
184+
|
185+
|I'll bump it to the DT maintainer queue. Thank you for your patience, @${author}.
186+
|
187+
|(Ping ${ownersToPing}.)`
188+
});

‎src/compute-pr-actions.ts‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -128,7 +128,7 @@ function extendPrInfo(info: PrInfo): ExtendedPrInfo {
128128
const needsAuthorAction = failedCI || info.hasMergeConflict || hasChangereqs;
129129
// => could be dropped from the extended info and replaced with: info.staleness?.kind === "Abandoned"
130130
const staleness = getStaleness();
131-
const reviewColumn = getReviewColumn();
131+
const reviewColumn = getReviewColumn(approverKind);
132132
return {
133133
...info, orig: info,
134134
authorIsOwner, editsInfra, possiblyEditsInfra, checkConfig,
@@ -142,15 +142,15 @@ function extendPrInfo(info: PrInfo): ExtendedPrInfo {
142142

143143
// Staleness timeline configurations (except for texts that are all in `comments.ts`)
144144
function getStaleness() {
145-
const ownersToPing = otherOwners.length === 0 ? ["«anyone?»"]
146-
: otherOwners.filter(o => !approvedReviews.some(r => o === r.reviewer));
145+
const ownersToPing = otherOwners.map(o => "@"+o).join(", ") || "«anyone?»";
147146
const mkStaleness = makeStaleness(info.now, info.author, ownersToPing);
148147
if (canBeSelfMerged) return info.mergeOfferDate && mkStaleness( // no merge offer yet: avoid the unreviewed timeline
149148
"Unmerged", info.mergeOfferDate, 4, 9, 30, "*REMOVE*");
150149
if (needsAuthorAction) return mkStaleness(
151150
"Abandoned", info.lastActivityDate, 6, 22, 30, "*REMOVE*");
152-
if (!approved) return mkStaleness(
153-
"Unreviewed", info.lastPushDate, 6, 10, 17, "Needs Maintainer Action");
151+
// Don't re-ping about PRs that have reviews and are already in the maintainer queue.
152+
if (approvedBy.length === 0 || approverKind !== "maintainer") return mkStaleness(
153+
"Unreviewed", info.lastPushDate, 6, 10, 17, getReviewColumn("maintainer"));
154154
return undefined;
155155
}
156156

@@ -192,7 +192,7 @@ function extendPrInfo(info: PrInfo): ExtendedPrInfo {
192192
&& (approverKind === "other" || approvedBy.includes("maintainer") || approvedBy.includes(approverKind));
193193
}
194194

195-
function getReviewColumn(): ColumnName {
195+
function getReviewColumn(approverKind: ApproverKind): ColumnName {
196196
// Get the project column for review with least access
197197
// E.g. let people review, but fall back to the DT maintainers based on the access rights above
198198
return approverKind !== "maintainer" ? "Waiting for Code Reviews"
@@ -339,7 +339,7 @@ export function process(prInfo: BotResult,
339339
// Has it: got no DT tests but is approved by DT modules and basically blocked by the DT maintainers - and it has been over 3 days?
340340
// Send a message reminding them that they can un-block themselves by adding tests.
341341
if (!info.hasTests && !info.hasMultiplePackages && info.approvedBy.includes("owner") && !info.editsInfra
342-
&& info.approverKind === "maintainer" && (info.staleness?.days ?? 0) > 3) {
342+
&& info.approverKind === "maintainer" && dayjs(info.now).diff(info.lastPushDate, "days") > 3) {
343343
post(Comments.RemindPeopleTheyCanUnblockPR(info.author, info.approvedReviews.map(r => r.reviewer),
344344
info.ciResult === "pass", headCommitAbbrOid));
345345
}
@@ -350,7 +350,7 @@ export function process(prInfo: BotResult,
350350
return actions;
351351
}
352352

353-
function makeStaleness(now: Date, author: string, ownersToPing: string[]) { // curried for convenience
353+
function makeStaleness(now: Date, author: string, ownersToPing: string) { // curried for convenience
354354
return (kind: StalenessKind, since: Date,
355355
freshDays: number, attnDays: number, nearDays: number,
356356
doneColumn: ColumnName) => {

0 commit comments

Comments
 (0)