refactor(lifecycle): adopt style guide rules around pub/module visibility - #4663
Merged
Conversation
…lity This adopts the new style guide rules around module visibility introduced in NVIDIA#4522, applying the correct visibility throughout the lifecycle-controller crates. Primary callouts are: - Replace 224 overbroad declarations across Machine-a-tron, Site Explorer, Machine Controller, State Controller, Preingestion Manager, and Machine Validation with private, `pub(super)`, or `pub(crate)` visibility based on actual callers. - Keep cross-crate controller contracts and explicit integration-test support public where their callers still need them. - Scope internal state processors, handlers, metrics, simulator helpers, and test fixtures to their actual modules. - Remove an unused State Controller test field exposed during dead-code cleanup. - Leave the generated Machine Controller declarations owned by VSC-01 unchanged and preserve runtime behavior. Tests updated! This supports NVIDIA#4553. 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>
Contributor
Author
|
@coderabbitai full_review, thanks! |
Contributor
|
🐇 ✅ ✅ Action performedFull review finished. |
Contributor
|
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 (28)
Summary by CodeRabbit
WalkthroughThe pull request narrows Rust visibility across internal crates. Public types, functions, methods, fields, constants, and test helpers now use ChangesInternal visibility scope
Estimated code review effort: 2 (Simple) | ~15 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
10 tasks
chet
enabled auto-merge (squash)
August 6, 2026 14:34
ianderson-nvidia
approved these changes
Aug 6, 2026
10 tasks
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.
This adopts the new style guide rules around module visibility introduced in #4522, applying the correct visibility throughout the lifecycle-controller crates.
Primary callouts are:
pub(super), orpub(crate)visibility based on actual callers.Tests updated!
Related issues
This supports #4553.
This supports the updated Rust visibility scoping guidelines in
STYLE_GUIDE.md, established in #4522.Type of Change
Breaking Changes
Testing
The targeted unit, integration, and doc-test suites for all six crates passed. I also ran:
make core/tests TEST_ARGS="-p carbide-machine-a-tron -p carbide-site-explorer -p carbide-machine-controller -p state-controller -p carbide-preingestion-manager -p carbide-machine-validation" cargo make format-nightly cargo make clippy cargo make carbide-lints cargo clippy --locked --all-targets --all-features -p carbide-machine-a-tron -p carbide-site-explorer -p carbide-machine-controller -p state-controller -p carbide-preingestion-manager -p carbide-machine-validation -- --force-warn unreachable-pub git diff --checkAdditional Notes
The initial compiler-backed visibility inventory identified 224 overbroad declarations across these six crates. The final 28-file diff resolves every tracked-source finding. The only remaining warnings are two generated Machine Controller declarations that are owned by VSC-01 and excluded from this issue.
Local CodeRabbit and read-only Claude reviews completed over the full diff. All applicable findings are incorporated.
Closes #4553