Skip to content

chore: remove new-pr label and 24 hour rule - #239

Merged
MarshallOfSound merged 1 commit into
mainfrom
sam/remove-new-pr-label
Sep 12, 2026
Merged

MarshallOfSound merged 1 commit into
mainfrom
sam/remove-new-pr-label

Conversation

@MarshallOfSound

Copy link
Copy Markdown
Member

Removes the new-pr 🌱 label and everything that existed to manage it (24 hour rule handler, its cron, exclusion lists, tests, fixtures, README section).

API review used that label as its minimum open time gate, so that gate is now time based instead. checkPRReadyForMerge compares against the same ready time the check title already shows ("ready on "): opened / last marked ready for review + 7 days.

Things that behave differently:

  • api-review/skip-delay ⏰ now actually skips the delay. Previously it only changed the date in the check title and the real bypass was fast-track / backport removing new-pr.
  • The 5 minute cron lives in setupAPIReviewStateManagement now and only looks at open PRs that have an api-review/* label.
  • The old cron paginated from page = 0 which GitHub treats as page 1, so the first 100 PRs were processed twice per run. Switched to octokit.paginate.

The new-pr 🌱 label itself will need deleting from electron/electron by hand after this deploys.

Removes the `new-pr 🌱` label and everything that existed to manage
it (24 hour rule handler, its cron, exclusion lists, tests, fixtures,
README section).

API review used that label as its minimum open time gate, so that
gate is now time based instead. `checkPRReadyForMerge` compares
against the same ready time the check title already shows ("ready on
<date>"): opened / last marked ready for review + 7 days.

Things that behave differently:
* `api-review/skip-delay ⏰` now actually skips the delay. Previously
  it only changed the date in the check title and the real bypass was
  `fast-track` / `backport` removing `new-pr`.
* The 5 minute cron lives in `setupAPIReviewStateManagement` now and
  only looks at open PRs that have an `api-review/*` label.
* The old cron paginated from `page = 0` which GitHub treats as page
  1, so the first 100 PRs were processed twice per run. Switched to
  `octokit.paginate`.

The `new-pr 🌱` label itself will need deleting from
electron/electron by hand after this deploys.
@MarshallOfSound
MarshallOfSound requested review from a team as code owners September 12, 2026 02:22
@MarshallOfSound
MarshallOfSound enabled auto-merge (squash) September 12, 2026 02:25

@nmggithub nmggithub left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Astra tells me of this one correctness issue:

  • [P2] Revoke existing approval when the delay applies againsrc/api-review-state.ts, lines 411–418. If a young PR is approved using api-review/skip-delay and that label is subsequently removed, this early return preserves api-review/approved and the successful check until day seven. It also ignores subsequent API declines or change requests during that period. Reset premature approval to requested, and allow negative review outcomes to take effect. I reproduced the stale approval with a one-day-old PR and a subsequent decline.

@MarshallOfSound
MarshallOfSound dismissed nmggithub’s stale review September 12, 2026 02:41

this is completely unrelated to the act of removing the new-pr feature and exists before this change. dismissing review

@MarshallOfSound
MarshallOfSound merged commit 8ee66f2 into main Sep 12, 2026
6 checks passed
@MarshallOfSound
MarshallOfSound deleted the sam/remove-new-pr-label branch September 12, 2026 03:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants