Repository navigation
fix(cli): skip telemetry ping and hook reminder in rtk rewrite - #4619
Open
viniciusdsandrade wants to merge 2 commits into
Open
viniciusdsandrade wants to merge 2 commits into
viniciusdsandrade wants to merge 2 commits into
Conversation
rtk rewrite is invoked on the hook path before every command with stderr redirected or discarded. Running the daily telemetry ping and hook status check on Commands::Rewrite adds startup overhead on the hot path and consumes the once-a-day hook reminder marker before a user-visible command can display it. Signed-off-by: Vinícius DS Andrade <[email protected]>
Verify that rtk rewrite neither prints the daily missing-hook reminder nor consumes the startup marker before a normal filtered command runs. Signed-off-by: Vinícius DS Andrade <[email protected]>
📊 Automated PR Analysis
SummaryAdds Commands::Rewrite to the exclusion lists for the daily telemetry ping and the hook-check warning in src/lib.rs, matching the existing exclusion for Commands::Hook, since rtk rewrite runs on the hot path before every shell command. Extends existing unit and integration tests to cover the rewrite subcommand's behavior. Review Checklist
Linked issues: #4615 Analyzed automatically by wshm · This is an automated analysis, not a human review. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Skip
core::telemetry::maybe_ping()andhooks::hook_check::maybe_warn()whenrtkis invoked asrtk rewrite, matching the existing exclusion forrtk hook.Closes #4615
Context & Problem
In
src/lib.rs,run_cli()skips the daily telemetry ping (pings_telemetry) and the once-a-day missing/outdated hook check (hooks::hook_check::maybe_warn()) forCommands::Hook { .. }, but still runs both forCommands::Rewrite { .. }.Shell hooks and plugin adapters invoke
rtk rewrite "$CMD" 2>/dev/nullbefore every command on the hot path. BecauseCommands::Rewriteis not excluded:rtk rewriteprints the warning tostderr(where the hook wrapper redirects it to/dev/nullor captures it in hook plumbing) and touches.hook_warn_last, silencing the reminder for the actual user-visible command that runs next.rtk rewriteruns the daily telemetry check and spawns a background ping thread on a short-lived process that exits immediately after printing the rewritten command.Solution
Commands::Rewrite { .. }topings_telemetry()insrc/lib.rs.Commands::Rewrite { .. }to the!matches!(cli.command, ...)guard beforehooks::hook_check::maybe_warn()insrc/lib.rs.hook_commands_skip_the_telemetry_pinginsrc/lib.rsandhook_entry_leaves_startup_warning_for_a_normal_commandintests/hook_warning_scope_test.rsto coverrtk rewrite.How to Test
Verified by static code inspection of
src/lib.rs,src/hooks/hook_check.rs, andtests/hook_warning_scope_test.rs; execution is delegated to the PR CI matrix:Checklist
cargo testpasses (pending CI execution; not run locally)cargo clippy --all-targetsclean (pending CI execution; not run locally)cargo fmt --checkclean (pending CI execution; not run locally)[\u{26a1} N tokens]badge (viaformat_badge()) (N/A: CLI startup guard fix)summary_footer()line (N/A: CLI startup guard fix)