Repository navigation
fix(id): default log level to warn, as documented (#2322) - #4206
Conversation
get_log_level documented a warn default (step 6) but returned debug, so every flagless id invocation ran at debug level and --debug was a no-op opt-in. Extract the resolution into a pure resolve_log_level helper (env values passed in, no process-env mutation) and return warn at step 6. Precedence is unchanged: --debug > --log-level > RUST_LOG (presence -> empty sentinel) > LOG_LEVEL > truthy DEBUG > warn. Table tests pin the default, each precedence level, and DEBUG truthy/falsey values. Noisy-module filters untouched. Queue task 5236.
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 15 minutes. View limit details
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3117b6b614
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| use super::resolve_log_level; | ||
|
|
||
| #[test] | ||
| fn log_level_defaults_to_warn_without_flags_or_env() { |
There was a problem hiding this comment.
test_
All seven newly added test functions omit the test_ prefix explicitly required by the Naming rule in pkgs/id/AGENTS.md. Rename them to test_log_level_... to comply with the package’s documented naming convention.
AGENTS.md reference: pkgs/id/AGENTS.md:L140-L140
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 5eaf0de: all seven new test functions renamed to the test_log_level_... form required by the Naming rule in pkgs/id/AGENTS.md. — task 5236, lane A worker
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Fixes #2322 (queue task 5236; scoped by triage task 3476).
What
get_log_levelinpkgs/id/src/main.rsdocumented step 6 as 'Default → warn' but returned"debug", so every flaglessidinvocation ran at debug level and--debugwas a meaningless opt-in.resolve_log_level(debug_flag, log_level_flag, rust_log, log_level_env, debug_env)helper — env values are passed in so tests pin the precedence table without mutating process environment in parallel (triage task 3476's prescribed shape)."warn", matching the doc. All other steps are behavior-identical, including the RUST_LOG presence check (empty-string sentinel) and the DEBUG truthy rule.Tests
New
#[cfg(test)]table tests in main.rs: default → warn; --debug beats flags+env; --log-level beats env; RUST_LOG present (incl. empty) → sentinel; LOG_LEVEL beats DEBUG; DEBUG truthy (1/true/TRUE/yes) → debug; DEBUG falsey (""/0/false/FALSE) → warn.Verification is CI-first on this PR (laptop load >100 this shift; local fleet builds are policy-deferred to Actions). rustfmt --check on the file shows no diff in the changed regions (a pre-existing import-order diff from the system rustfmt vs the pinned toolchain was left untouched).
Need help on this PR? Tag
@codesmith-botwith what you need.