PR #451 says Closes #439 but only covers one slice -- split it? #452

New issue

Closed

opened 2026-07-28 23:22:35 +00:00 by igor · 1 comment

igor commented

2026-07-28 23:22:35 +00:00

Collaborator

The shadow review of PR #451 blocked on something I can't fix from a rework
worktree, so it needs you.

#451's body says Closes #439. It shouldn't. #439 defines the whole needs-you
set, and #451 implements one source of it: "Igor requested your review on a PR",
delivered as one email per PR per head. Merging it as-is auto-closes #439 and
takes the rest of the ticket with it.

What's actually landed against #439 so far:

#449 (merged) -- detection. lib/needsyou.sh builds the waiting set from PRs
and from issues carrying Status/Blocked / Status/Need More Info, tracks
since for wait time, renders grouped, and treats removals as silent. It only
LOGS today.
#451 (this one) -- one event-driven email, hooked at forgejo_request_review.

What's left:

Delivery for the needsyou set itself -- #449 detects and logs; nothing mails.
Two sources #449 doesn't scan: CEO proposals / bot questions awaiting your
greenlight, and failed systemd units.
The removal is silent and second distinct item notifies tests #439 names,
which only mean something once there's a delivery path to test them against.

Two things I'd like from you:

Edit #451's body to say Part of #439 before merging it. The rework path
doesn't regenerate a PR body and I have no sanctioned way to PATCH one, so
this is a manual edit or it ships as-is.
Split #439 into the slices above (it's already Status/Blocked with "Split
this into smaller issues"), so what's left survives #439 being closed.

Happy to file the split tickets myself if you'd rather -- say the word and I'll
enqueue them.

The shadow review of PR #451 blocked on something I can't fix from a rework worktree, so it needs you. #451's body says `Closes #439`. It shouldn't. #439 defines the whole needs-you set, and #451 implements one source of it: "Igor requested your review on a PR", delivered as one email per PR per head. Merging it as-is auto-closes #439 and takes the rest of the ticket with it. What's actually landed against #439 so far: - #449 (merged) -- detection. `lib/needsyou.sh` builds the waiting set from PRs and from issues carrying `Status/Blocked` / `Status/Need More Info`, tracks `since` for wait time, renders grouped, and treats removals as silent. It only LOGS today. - #451 (this one) -- one event-driven email, hooked at `forgejo_request_review`. What's left: - Delivery for the needsyou set itself -- #449 detects and logs; nothing mails. - Two sources #449 doesn't scan: CEO proposals / bot questions awaiting your greenlight, and failed systemd units. - The `removal is silent` and `second distinct item notifies` tests #439 names, which only mean something once there's a delivery path to test them against. Two things I'd like from you: 1. Edit #451's body to say `Part of #439` before merging it. The rework path doesn't regenerate a PR body and I have no sanctioned way to PATCH one, so this is a manual edit or it ships as-is. 2. Split #439 into the slices above (it's already `Status/Blocked` with "Split this into smaller issues"), so what's left survives #439 being closed. Happy to file the split tickets myself if you'd rather -- say the word and I'll enqueue them.

igor added the

Status

Need More Info

label

2026-07-28 23:22:36 +00:00

joshtronic was assigned by igor

2026-07-28 23:22:36 +00:00

igor commented

2026-07-28 23:43:57 +00:00

Author

Collaborator

Done — #451's body now reads Part of #439 and explains why. The review was right: Closes was mine and it over-claimed.

Request 2 (splitting #439) is a scope decision, not a ticket-mechanics one, and it is going to the operator directly rather than sitting here as a second open issue. Closing so the queue reflects reality.

Done — #451's body now reads `Part of #439` and explains why. The review was right: `Closes` was mine and it over-claimed. Request 2 (splitting #439) is a scope decision, not a ticket-mechanics one, and it is going to the operator directly rather than sitting here as a second open issue. Closing so the queue reflects reality.

igor closed this issue

2026-07-28 23:43:58 +00:00

igor referenced this issue from a commit

2026-07-28 23:45:34 +00:00

fix: pin the notifier's delivery seam and cover the new sources