fix(dylint): clean up violations after dylint-link rollout#991
Conversation
|
Warning Review limit reached
Next review available in: 19 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR migrates path handling across compile backends, ESP32 orchestrator, deploy tool discovery, and esptool package installation from ChangesNormalizedPath migration and path-key normalization
Estimated code review effort: 3 (Moderate) | ~30 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/fbuild-cli/src/lib_select.rs (1)
341-353: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winAdd coverage for
normalized_path_is_under.crates/fbuild-cli/tests/lib_select.rsonly covers CLI parsing and early exits, so the new path-prefix logic behindtriggered_bystill lacks direct tests. Cover exact match, trailing-slash prefix matching, and the/foo/barbazvs/foo/barnegative case.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/fbuild-cli/src/lib_select.rs` around lines 341 - 353, Add direct test coverage for normalized_path_is_under in lib_select.rs, since the current tests only exercise CLI parsing and early exits. Update crates/fbuild-cli/tests/lib_select.rs to verify the helper’s exact-match behavior, prefix matching when the directory has or needs a trailing slash, and the negative case where /foo/barbaz must not match /foo/bar. Use normalized_path_is_under and triggered_by as the key symbols to locate the path-prefix logic.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/fbuild-deploy/src/lpc_debugger_reflash.rs`:
- Around line 277-286: `home_dir()` only reads `USERPROFILE` on Windows, so add
a `HOME` fallback there to match `probe_rs.rs::home_dir_local()`. Update the
Windows branch in `home_dir()` to try `USERPROFILE` first and then `HOME` if
needed, so callers like `managed_tools_dir()`, `find_dfu_util()`, and
`find_lpc_link2_firmware()` still resolve a home directory in shells that only
set `HOME`.
---
Nitpick comments:
In `@crates/fbuild-cli/src/lib_select.rs`:
- Around line 341-353: Add direct test coverage for normalized_path_is_under in
lib_select.rs, since the current tests only exercise CLI parsing and early
exits. Update crates/fbuild-cli/tests/lib_select.rs to verify the helper’s
exact-match behavior, prefix matching when the directory has or needs a trailing
slash, and the negative case where /foo/barbaz must not match /foo/bar. Use
normalized_path_is_under and triggered_by as the key symbols to locate the
path-prefix logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3a043b48-f7ea-4cdf-8f61-3cb522537c2c
⛔ Files ignored due to path filters (1)
dylints/ban_std_pathbuf/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (12)
crates/fbuild-build/src/compile_backend.rscrates/fbuild-build/src/compiler.rscrates/fbuild-build/src/compiler_tests.rscrates/fbuild-build/src/esp32/orchestrator/boot_artifacts.rscrates/fbuild-build/src/esp32/orchestrator/build.rscrates/fbuild-build/src/esp32/orchestrator/packages.rscrates/fbuild-cli/src/lib_select.rscrates/fbuild-deploy/src/lpc_debugger_reflash.rscrates/fbuild-deploy/src/probe_rs.rscrates/fbuild-packages/src/library/esptool.rscrates/fbuild-packages/src/library/library_compiler.rsdylints/ban_direct_serialport/src/allowlist.txt
Summary
Closes #964. Advances #990.
Local validation
Summary by CodeRabbit
--explainand--jsonoutput.