refactor(bmc/console): adopt style guide rules for pub/module visibility - #4662
Conversation
This adopts the new style guide rules around module visibility introduced in NVIDIA#4522, applying the correct visibility throughout the BMC and console group. Primary callouts are: - Replace unrestricted `pub` across `ssh-console`, `bmc-explorer`, `bmc-proxy`, `ipmi`, `host-support`, `redfish`, and `nvue-client` with private, `pub(super)`, `pub(crate)`, or named-ancestor visibility based on actual callers. - Keep the BMC explorer and Redfish test-support surfaces public where downstream crate and integration-test callers rely on them. - Restrict ssh-console's BMC connection tree to its parent module while keeping frontend messages and vendor types available at the crate boundaries that use them. - Make binary entrypoints, configuration internals, implementation types, helper methods, and test utilities visible only where their callers need them. - Preserve public library APIs, error surfaces, serialized configuration, test fixtures, and runtime behavior. Tests updated! This supports NVIDIA#4554. This supports the updated Rust visibility scoping guidelines in `STYLE_GUIDE.md`, established in NVIDIA#4522. Signed-off-by: Chet Nichols III <chetn@nvidia.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (43)
Summary by CodeRabbit
WalkthroughThe PR narrows public Rust APIs to crate, parent-module, or private visibility across BMC exploration, proxy, host-support, IPMI, Redfish, NVUE, SSH-console, and test-support code. Runtime behavior and method signatures remain unchanged. ChangesInternal API visibility refactor
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full_review, thanks! |
|
ᕱᕱ
|
|
@coderabbitai full_review, thanks! |
|
🐇 ✅ Action performedFull review finished. |
This adopts the new style guide rules around module visibility introduced in #4522, applying the correct visibility throughout the BMC and console group.
Primary callouts are:
pubacrossssh-console,bmc-explorer,bmc-proxy,ipmi,host-support,redfish, andnvue-clientwith private,pub(super),pub(crate), or named-ancestor visibility based on actual callers.Tests updated!
Related issues
This supports #4554.
This supports the updated Rust visibility scoping guidelines in
STYLE_GUIDE.md, established in #4522.Type of Change
Breaking Changes
Testing
All 208 touched-crate unit and integration tests passed, including the
ssh-consoleintegration suite. I also ran:cargo test --locked --all-features -p carbide-ssh-console -p bmc-explorer -p carbide-bmc-proxy -p carbide-ipmi -p carbide-host-support -p carbide-redfish -p nvue-client cargo clippy --locked --all-targets --all-features -p carbide-ssh-console -p bmc-explorer -p carbide-bmc-proxy -p carbide-ipmi -p carbide-host-support -p carbide-redfish -p nvue-client --message-format short -- --force-warn unreachable-pub cargo make format-nightly cargo make clippy cargo make carbide-lints git diff --checkAdditional Notes
The initial visibility lint identified 192 overbroad declarations across 40 files. Narrowing those parent items exposed 145 contained fields and helpers whose visibility could move with them; the final 43-file diff tightens 337 declarations. The unrestricted public surfaces left are backed by cross-crate callers, integration-test consumers, or intentional root re-exports.
Local CodeRabbit, read-only Claude, and self-reviews completed over the full diff. All applicable findings are incorporated.
Closes #4554