Repository navigation
Conversation
Without the `tracing` feature the `debug!` macro expands to nothing, so the `label(operation)` in its arguments is never referenced and clippy under `--no-default-features` fails the build on a dead function and an unused import. That step (#155) and the span work (#165) each passed CI on their own and were merged within minutes of each other, which is how main went red. `label` now exists only with the feature, its one-line `is_raw` helper is folded in so the `OperationInfo` import can go, and the call sites name it fully qualified the way they already name the macro, so the import line needs no cfg of its own.
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. |
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped to feature-gating and import hygiene and align with how the trace::debug! macro already compiles to nothing without the tracing feature.
Pull request overview
This PR fixes --no-default-features Clippy failures by ensuring tracing-only helpers aren’t compiled (or imported) when the tracing feature is disabled, while keeping the call sites uniform.
Changes:
- Gate
trace::labelbehind#[cfg(feature = "tracing")]and inline the formeris_rawlogic. - Remove the now-unneeded
OperationInfo/labelimports and referencelabelwith a fully-qualified path at the (tracing) call sites.
[!TIP]
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or rungh pr ready --undo.
Click "Ready for review" or rungh pr readyto reengage.
File summaries
| File | Description |
|---|---|
| rust/hey-sdk/src/trace.rs | Gates label behind the tracing feature and removes the redundant is_raw helper/import. |
| rust/hey-sdk/src/client.rs | Drops the label import and fully-qualifies crate::trace::label(...) within tracing debug calls. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Closing: #163 landed the same fix while this was open ( |
Main has been red on Rust Tests since #165 merged:
cargo clippy --no-default-features -D warningsfails ontrace::labelandtrace::is_rawbeing dead and thelabelimport inclient.rsunused. Without thetracingfeature thedebug!macro expands to nothing, so thelabel(operation)in its arguments is never referenced.#155 added that clippy step and #165 added the spans. Each was green on its own; #165's CI ran before #155 landed and it merged without a re-run.
The fix gates
labelbehind the feature, folds the one-lineis_rawhelper into it so theOperationInfoimport can go, and nameslabelfully qualified at the three call sites the way they already name the macro, so the import line needs no cfg.make rs-check rs-check-driftis green locally. #171 (the zizmor-action bump) inherits this failure through its merge commit and will need a rebase once this lands.Summary by cubic
Fixes the
cargo clippy --no-default-features -D warningsfailure on main by gating the tracing label behind thetracingfeature.is_rawhelper intolabelso the unusedOperationInfoimport goes away.labelfully qualified at call sites so the import line needs no cfg.Written for commit 76144fe. Summary will update on new commits.