Skip to content

ci: draft inactive PRs awaiting author action - #26715

Merged
kumarUjjawal merged 2 commits into
apache:mainfrom
kumarUjjawal:ci/draft-inactive-prs
Oct 25, 2026
Merged

kumarUjjawal merged 2 commits into
apache:mainfrom
kumarUjjawal:ci/draft-inactive-prs

Conversation

@kumarUjjawal

@kumarUjjawal kumarUjjawal commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Related to #26684.

Rationale for this change

Keep the active review queue focused on PRs that can progress.

What changes are included in this PR?

Automatically draft PRs inactive for 14 days with merge conflicts, failing CI, or changes requested on the latest push. Includes a brief explanation and a manual dry-run mode.

What is the testing strategy for this PR?

Validated with workflow lint and a live dry run (69 eligible PRs); no automated tests added.

Are there any user-facing changes?

Authors can mark their PR ready for review again. The draft notification restarts the existing 60-day stale timer.

@github-actions github-actions Bot added documentation Improvements or additions to documentation development-process Related to development process of DataFusion labels Oct 24, 2026

@jayzhan211 jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The drafting rule looks right to me. The main problem is that conflicts are mostly missed on a cold run; details inline.

Comment thread ci/scripts/draft_inactive_prs.cjs Outdated
per_page: 100,
});
const results = [];
for (const listed of prs) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GitHub computes mergeable lazily, so the first query for a PR usually returns UNKNOWN, which this treats as "no conflict". I dry-ran the script twice against the repo: the first run found 9 conflicting PRs (27 eligible in total) and the second found 60 (69 in total). Since the cron usually runs after main has moved, most conflicts would be missed, and which PRs get drafted would depend on whether something else queried them first. Could we warm the state up with one pass and a short wait before deciding? With this change the dry run gives the same 69 PRs from a cold start:

-  const results = [];
-  for (const listed of prs) {
-    if (listed.draft || !(Date.parse(listed.updated_at) <= cutoff)) continue;
+  const candidates = prs.filter(
+    (listed) => !listed.draft && Date.parse(listed.updated_at) <= cutoff
+  );
+  // GitHub computes `mergeable` lazily: the first query starts the job and
+  // returns UNKNOWN, so query every candidate once and wait before deciding.
+  for (const listed of candidates) {
+    await loadPullRequest(github, repo, listed.number);
+  }
+  await setTimeout(30_000);
+  const results = [];
+  for (const listed of candidates) {
     let pr = await loadPullRequest(github, repo, listed.number);

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sounds good

Comment thread ci/scripts/draft_inactive_prs.cjs Outdated
)
reasons.push("failing CI checks");

const forcePush = pr.timelineItems.nodes[0]?.createdAt;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: review.commit?.oid === pr.headRefOid already means nothing was pushed after the review, so the force-push and pushedDate checks aren't needed. pushedDate is also deprecated (its removal date was 2023-07-01) and returns null for every PR I checked, and if GitHub removes it the whole query will fail. Could we drop it together with timelineItems and submittedAt? The dry run gives the same results without them:

-  const forcePush = pr.timelineItems.nodes[0]?.createdAt;
-  const pushedDate = pr.commits.nodes[0]?.commit.pushedDate;
-  // pushedDate can be null; also check the reviewed commit and last force push.
   if (
     pr.reviews.some(
       (review) =>
         review.state === "CHANGES_REQUESTED" &&
-        review.commit?.oid === pr.headRefOid &&
-        (!forcePush ||
-          Date.parse(review.submittedAt) > Date.parse(forcePush)) &&
-        (!pushedDate || Date.parse(review.submittedAt) > Date.parse(pushedDate))
+        review.commit?.oid === pr.headRefOid
     )
   )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call

@kumarUjjawal
kumarUjjawal force-pushed the ci/draft-inactive-prs branch from d3644c5 to ef5709f Compare October 25, 2026 05:25
@kumarUjjawal

Copy link
Copy Markdown
Contributor Author

Thanks @jayzhan211

Pushed the changes

@jayzhan211 jayzhan211 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @kumarUjjawal

@kumarUjjawal
kumarUjjawal added this pull request to the merge queue Oct 25, 2026
Merged via the queue into apache:main with commit e4b92af Oct 25, 2026
46 of 47 checks passed
@kumarUjjawal
kumarUjjawal deleted the ci/draft-inactive-prs branch October 25, 2026 14:46
@alamb

alamb commented Oct 25, 2026

Copy link
Copy Markdown
Contributor

Thank you for this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

development-process Related to development process of DataFusion documentation Improvements or additions to documentation v56.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants