-
Notifications
You must be signed in to change notification settings - Fork 3.6k
Budget the editor-handoff pause sleep, and note why main looked green without it #6243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -451,6 +451,9 @@ | |
| "crates/tui/src/tui/ui/provider_setup.rs": { | ||
| "std_fs": 2 | ||
| }, | ||
| "crates/tui/src/tui/ui/terminal_input.rs": { | ||
| "thread_sleep": 1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [INFO] Rationale for the budgeted exception lives only outside the repository The justification quoted in the PR (bounded retry capped by TERMINAL_INPUT_CHILD_PAUSE_TIMEOUT, synchronous API whose caller blocks the thread on a foreground editor) is the reason the budget is raised instead of the call being rewritten, but it will not be discoverable from the budget file, which is plain JSON with no comment support. A reader investigating later why this file carries a non-zero thread_sleep budget will have to dig through git history to find it. Nothing to fix in this diff, but noting it so the tradeoff is at least acknowledged. |
||
| }, | ||
| "crates/tui/src/tui/ui/tests.rs": { | ||
| "std_fs": 3, | ||
| "thread_sleep": 1 | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[INFO] Verify the key is not already present elsewhere in the budget file
The diff shows only the insertion context around line 451, so a pre-existing entry for "crates/tui/src/tui/ui/terminal_input.rs" elsewhere in the JSON cannot be ruled out from the patch alone. JSON parsers silently keep the last duplicate key while the generator (--update) may emit only one, so a duplicate would produce confusing future diffs and a budget that does not match what the author intended. Since the file is sorted alphabetically, a duplicate should be adjacent (or would break the ordering assumption), which makes this cheap to confirm with a quick grep before merge.