This is an automated email from the ASF dual-hosted git repository.
derrickaw pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/beam.git
The following commit(s) were added to refs/heads/master by this push:
new 5a218f6c241 assign reviewers immediately when PR is marked ready for
review (#40400)
5a218f6c241 is described below
commit 5a218f6c241b798fedef9aebdbf5efd50244711f
Author: Derrick Williams <[email protected]>
AuthorDate: Mon Oct 5 09:52:27 2026 -0400
assign reviewers immediately when PR is marked ready for review (#40400)
* assign reviewers immediately when PR is marked ready for review
* add comments
---
scripts/ci/pr-bot/README.md | 1 +
scripts/ci/pr-bot/processNewPrs.ts | 19 ++++++++++++++-----
scripts/ci/pr-bot/processPrUpdate.ts | 10 +++++++++-
scripts/ci/pr-bot/shared/checks.ts | 7 ++++---
4 files changed, 28 insertions(+), 9 deletions(-)
diff --git a/scripts/ci/pr-bot/README.md b/scripts/ci/pr-bot/README.md
index 6cb9f0a5fcc..c5c30e9c5c2 100644
--- a/scripts/ci/pr-bot/README.md
+++ b/scripts/ci/pr-bot/README.md
@@ -37,6 +37,7 @@ The bot consists of three core workflows and a persistent
state tracking system:
### 2. PR Updates & Commands (`processPrUpdate.ts`)
* Triggered on PR pushes (`synchronize`), draft transitions
(`converted_to_draft`, `ready_for_review`), and comments (`issue_comment:
created`).
* Shifts attention to author (`Next Action: Author`) when a PR is marked as
draft (`converted_to_draft`) and back to reviewers (`Next Action: Reviewers`)
when taken out of draft (`ready_for_review`) or when the author pushes new
commits or posts comments on a non-draft PR.
+* When a PR is marked `ready_for_review` and has no reviewers assigned yet,
immediately assigns reviewers if it already has matching reviewer labels and
passing CI checks.
* Removes `slow-review` label upon receiving a comment from a non-author
reviewer.
* Processes commands like `assign to next reviewer`, `waiting on author`,
`stop reviewer notifications`, `assign set of reviewers`, and `remind me after
tests pass`.
diff --git a/scripts/ci/pr-bot/processNewPrs.ts
b/scripts/ci/pr-bot/processNewPrs.ts
index fbcd78a0db8..2f24f8e08f4 100644
--- a/scripts/ci/pr-bot/processNewPrs.ts
+++ b/scripts/ci/pr-bot/processNewPrs.ts
@@ -44,7 +44,7 @@ import { CheckStatus } from "./shared/checks";
* unless we're supposed to remind the user after tests pass
* (in which case that's all we need to do).
*/
-function needsProcessed(pull: any, prState: typeof Pr): boolean {
+export function needsProcessed(pull: any, prState: typeof Pr): boolean {
if (github.hasLabel(pull, AWAITING_TRIAGE_LABEL)) {
console.log(
`Skipping PR ${pull.number} because it has awaiting triage label`
@@ -75,8 +75,13 @@ function needsProcessed(pull: any, prState: typeof Pr):
boolean {
console.log(`Skipping PR ${pull.number} because it is a WIP`);
return false;
}
+ // Wait 20 minutes before processing unlabeled PRs so the LabelPrs workflow
+ // has time to apply path-based labels before falling back to
no-matching-label.
let timeCutoff = new Date(new Date().getTime() - 20 * 60000);
- if (new Date(pull.created_at) > timeCutoff) {
+ if (
+ (!pull.labels || pull.labels.length === 0) &&
+ new Date(pull.created_at) > timeCutoff
+ ) {
console.log(
`Skipping PR ${pull.number} because it was created less than 20 minutes
ago`
);
@@ -186,7 +191,7 @@ async function isAnyGithubReviewerCommitter(pull: any):
Promise<boolean> {
return false;
}
-async function processPull(
+export async function processPull(
pull: any,
reviewerConfig: typeof ReviewerConfig,
stateClient: typeof PersistentState
@@ -365,7 +370,7 @@ async function processPull(
Object.values(prState.reviewersAssignedForLabels)
);
- github.nextActionReviewers(pull.number, pull.labels);
+ await github.nextActionReviewers(pull.number, pull.labels);
prState.nextAction = "Reviewers";
prState.reviewersAssignedAt = Date.now();
@@ -397,6 +402,10 @@ async function processNewPrs() {
}
}
-processNewPrs();
+// Only run processNewPrs() when executed directly so other modules (e.g.
processPrUpdate)
+// can import helper functions like processPull without scanning all open PRs.
+if (require.main === module) {
+ processNewPrs();
+}
export {};
diff --git a/scripts/ci/pr-bot/processPrUpdate.ts
b/scripts/ci/pr-bot/processPrUpdate.ts
index c5b0cf2905b..ad0aeaef80e 100644
--- a/scripts/ci/pr-bot/processPrUpdate.ts
+++ b/scripts/ci/pr-bot/processPrUpdate.ts
@@ -27,6 +27,7 @@ const {
getPullAuthorFromPayload,
getPullNumberFromPayload,
} = require("./shared/githubUtils");
+const { processPull } = require("./processNewPrs");
const { PersistentState } = require("./shared/persistentState");
const { ReviewerConfig } = require("./shared/reviewerConfig");
const {
@@ -225,7 +226,14 @@ async function processPrUpdate() {
await setNextActionAuthor(payload, pull, stateClient);
} else if (payload.action === "ready_for_review") {
console.log("Processing ready_for_review action");
- await setNextActionReviewers(payload, pull, stateClient);
+ // If reviewers are already assigned, shift attention back to them.
+ // Otherwise, try to assign initial reviewers immediately (e.g. when a
draft
+ // PR with passing checks is marked ready for review) instead of
waiting for cron.
+ if (await areReviewersAssigned(pull, stateClient)) {
+ await setNextActionReviewers(payload, pull, stateClient);
+ } else {
+ await processPull(pull, reviewerConfig, stateClient);
+ }
}
// TODO(damccorm) - it would be good to eventually handle the following
events here, even though they're not part of the normal workflow
// review requested, assigned, label added, label removed
diff --git a/scripts/ci/pr-bot/shared/checks.ts
b/scripts/ci/pr-bot/shared/checks.ts
index 187ff5771f9..9a43c90daf8 100644
--- a/scripts/ci/pr-bot/shared/checks.ts
+++ b/scripts/ci/pr-bot/shared/checks.ts
@@ -104,9 +104,10 @@ async function getChecksByName(
}
// Returns checks we should exclude because they are flaky or not always
predictive of pr mergability.
-// Currently just excludes codecov.
-function shouldExcludeCheck(check): boolean {
- if (check.name.toLowerCase().indexOf("codecov") != -1) {
+// Currently excludes codecov and the pr-bot update check itself (which may be
in_progress when checking status).
+export function shouldExcludeCheck(check: { name: string }): boolean {
+ const name = check.name.toLowerCase();
+ if (name.indexOf("codecov") != -1 || name === "process-pr-update") {
return true;
}
return false;
