chore(cli): route fbuild-cli through fbuild-paths; document the clap exception - #1393
Merged
Conversation
…exception Fifth ratchet batch for #1349. Allowlist 28 -> 20, and `fbuild-cli` is down to one entry that cannot be removed by editing code. Eight files sanitized: IDE state path and its two fixtures, the dev-mode and `--build-dir` notices, the purge usage line, the symbols ELF-search diagnostic and its fixture, `lib_select`'s skip-list arm, the debug/ide-debug ELF fixtures, and the crash-recovery test's lock path. Two of these stop being merely cosmetic. The dev-mode notice hardcoded `port 8865, ~/.fbuild/dev/`; it now prints `get_daemon_port()` and `get_fbuild_root()`, and the port has not been a fixed 8865 since #1009 made it version- and identity-derived. The symbols "no ELF found" diagnostic listed the directories it searched, spelled by hand — a list that can drift from the search it describes. ## `cli/args.rs` moves to the sanctioned section rather than the baseline It is not a hand-written literal. `#[derive(Subcommand)]` stringifies the `///` help text into `&str` literals whose `source_callsite()` is the derive item, so the finding lands on the file as a whole and no expression edit can clear it. The only alternative is deleting `.fbuild/build/**/firmware.elf` from help a user reads, which degrades the documentation to satisfy a lint. Verified rather than assumed: removing the line makes the lint fire at `args.rs:121`, which is the `#[derive(Subcommand)]` — exactly the macro case the lint's own docs call out. Lint crate bumped 0.1.4 -> 0.1.5 so the cached `.so` is rebuilt against the shrunk list. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Warning Review limit reachedNext included review available in 43 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (10)
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. Comment |
This was referenced Aug 24, 2026
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.
Fifth ratchet batch for #1349. Allowlist 28 → 20, and
fbuild-cliis downto one entry that cannot be removed by editing code.
Eight files sanitized
IDE state path and its two fixtures, the dev-mode and
--build-dirnotices,the purge usage line, the symbols ELF-search diagnostic and its fixture,
lib_select's skip-list arm, the debug / ide-debug ELF fixtures, and thecrash-recovery test's lock path.
Two of these stop being cosmetic:
port 8865, ~/.fbuild/dev/. The port hasnot been a fixed 8865 since daemon: endpoint isolation follow-up to #940 — key the default endpoint by cache-identity/checkout + deterministic version arbitration #1009 made it version- and identity-derived, so
this line has been telling users the wrong port. It now prints
get_daemon_port()andget_fbuild_root().spelled by hand — a list that can drift from the search it describes, which
is the worst kind of diagnostic to get wrong.
cli/args.rsmoves to the sanctioned section, not the baselineIt is not a hand-written literal.
#[derive(Subcommand)]stringifies the///help text into
&strliterals whosesource_callsite()is the derive item, sothe finding lands on the file as a whole and no expression edit can clear
it. The only way to remove it is deleting
.fbuild/build/**/firmware.elffrom help a user actually reads — degradingdocumentation to satisfy a lint.
Verified rather than assumed: removing the allowlist line makes the lint fire
at
args.rs:121, which is the#[derive(Subcommand)]itself. That is exactlythe macro case the lint's own "Known problems" section calls out, so it is a
documented exception rather than debt.
I moved it above the baseline marker with that rationale inline, so "the
allowlist reaches zero" stays an honest goal — the remaining entries are
things that can be fixed.
Verification
dylint --lib ban_raw_fbuild_path -- --workspace --all-targets: 0 findingssoldr cargo test -p fbuild-cli --bins: 305 passed-D warnings: clean.sois rebuilt against the shrunklist (checked against main's version, not against my own starting point —
that bump was a silent no-op last batch)
Remaining
20 entries: fbuild-build 12, fbuild-build-engine 5 (test fixtures), plus the 3
sanctioned sites (
fbuild-paths/lib.rs,fbuild-core/path.rs,fbuild-cli/args.rs). So 17 removable.