Skip to content

docs(admin-cli): clarify expected machine option behavior - #4898

Merged
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-4894
Aug 13, 2026
Merged

docs(admin-cli): clarify expected machine option behavior#4898
chet merged 1 commit into
NVIDIA:mainfrom
chet:gh-issue-4894

Conversation

@chet

@chet chet commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Keep Expected Machine help grammatical in both the live CLI and generated reference pages.

The roff-to-Markdown conversion drops apostrophes, so phrases such as machine's render incorrectly. This rephrases those options without apostrophes and documents the actual initial-only pause semantics plus the different DPF behavior for add and patch.

Related issues

Type of Change

  • Add - New feature or capability
  • Change - Changes in existing functionality
  • Fix - Bug fixes
  • Remove - Removed features or deprecated functionality
  • Internal - Internal changes (refactoring, tests, docs, etc.)

Breaking Changes

  • This PR contains breaking changes

Testing

  • Unit tests added/updated
  • Integration tests added/updated
  • Manual testing performed
  • No testing required (docs, internal refactor, etc.)

Additional Notes

Verified the live nico-admin-cli expected-machine add --help and patch --help contracts, then rendered the affected pages through the Bookworm roff/Pandoc pipeline. The generated reference updates remain isolated in the docs-only #4418 PR.

Validation also included nightly formatting, full workspace Clippy, custom Carbide lints, and the focused nico-admin-cli tests.

@chet
chet requested a review from a team as a code owner August 12, 2026 20:19
@chet

chet commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai full_review, thanks!

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Summary by CodeRabbit

  • Documentation
    • Updated CLI help text for default pause behavior, DPF settings, BMC IP allocation, and interface replacement.
    • Clarified configuration guidance without changing command behavior.

Walkthrough

The pull request updates expected machine add and patch CLI help text. It clarifies pause behavior, DPF defaults and preservation, BMC IP allocation scope, and interface replacement semantics. Parsing and conversion behavior remain unchanged.

Changes

Expected Machine CLI Help

Layer / File(s) Summary
Clarify expected machine option help
crates/admin-cli/src/expected_machines/add/args.rs, crates/admin-cli/src/expected_machines/patch/args.rs
The help text now documents pause behavior, DPF preservation, BMC IP allocation scope, and full interface-list replacement.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Possibly related issues

Possibly related PRs

  • NVIDIA/infra-controller#4842 — This PR updates help text in the same add and patch argument declarations, including BMC address documentation.
  • NVIDIA/infra-controller#4848 — This PR updates Expected Machine help text in the same argument files, including --interfaces patch behavior.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: clarifying Expected Machine option help text in the admin CLI.
Description check ✅ Passed The description directly explains the help-text changes, rendering issue, command-specific behavior, and validation performed.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands.

@coderabbitai

coderabbitai Bot commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

@chet I will perform a complete review of the pull request.

🐇

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
crates/admin-cli/src/expected_machines/add/args.rs (1)

154-162: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Regenerate the expected-machine reference pages.

The after_long_help examples exist in both Args structs, but both committed Markdown pages lack the EXAMPLES: sections and still contain stale help text. Regenerate the add and patch pages.

🤖 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/admin-cli/src/expected_machines/add/args.rs` around lines 154 - 162,
Regenerate the expected-machine reference Markdown pages from both Args structs
so they include the existing after_long_help EXAMPLES sections and current help
text. Apply this to crates/admin-cli/src/expected_machines/add/args.rs lines
154-162 and 193, and crates/admin-cli/src/expected_machines/patch/args.rs lines
171-179, 213, and 222; update the corresponding add and patch committed Markdown
pages, with no direct source change required at these sites.

Sources: Coding guidelines, Path instructions

🤖 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.

Nitpick comments:
In `@crates/admin-cli/src/expected_machines/add/args.rs`:
- Around line 154-162: Regenerate the expected-machine reference Markdown pages
from both Args structs so they include the existing after_long_help EXAMPLES
sections and current help text. Apply this to
crates/admin-cli/src/expected_machines/add/args.rs lines 154-162 and 193, and
crates/admin-cli/src/expected_machines/patch/args.rs lines 171-179, 213, and
222; update the corresponding add and patch committed Markdown pages, with no
direct source change required at these sites.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: 2ecfb887-a706-4f95-8e79-23d8343e14f1

📥 Commits

Reviewing files that changed from the base of the PR and between 6c92b73 and 88e1407.

📒 Files selected for processing (2)
  • crates/admin-cli/src/expected_machines/add/args.rs
  • crates/admin-cli/src/expected_machines/patch/args.rs

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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/admin-cli/src/expected_machines/add/args.rs`:
- Around line 154-162: Regenerate the expected-machine reference pages using the
repository’s gen-cli-docs task with the Bookworm/Pandoc toolchain, ensuring all
eight add examples and seven patch examples are included. Commit the resulting
updates to both generated reference pages without modifying the Rust help
definitions.
🪄 Autofix

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: Enterprise

Run ID: dcb2fd1e-81df-46c7-a079-6bd02416c76b

📥 Commits

Reviewing files that changed from the base of the PR and between 6c92b73 and 88e1407.

📒 Files selected for processing (2)
  • crates/admin-cli/src/expected_machines/add/args.rs
  • crates/admin-cli/src/expected_machines/patch/args.rs

Comment on lines +154 to +162
help = "Initial pause state applied when the BMC endpoint for this machine is first explored. `true` pauses ingestion and automatic power-on; `false` pauses neither. Defaults to `false`."
)]
default_pause_ingestion_and_poweron: Option<bool>,

#[clap(
long,
action = clap::ArgAction::Set,
value_name = "DPF_ENABLED",
help = "DPF enable/disable for this machine. Default is updated as true.",
help = "Whether DPF is enabled for this machine. Defaults to true.",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

for file in \
  crates/admin-cli/src/expected_machines/add/args.rs \
  crates/admin-cli/src/expected_machines/patch/args.rs
do
  echo "== $file =="
  rg -n -B8 -A8 'after_long_help|EXAMPLES:|struct Args' "$file"
done

fd -t f . | rg 'expected-machine-(add|patch)\.(md|roff)$'

Repository: NVIDIA/infra-controller

Length of output: 3490


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== add help block =="
sed -n '28,75p' crates/admin-cli/src/expected_machines/add/args.rs

echo "== patch help block =="
sed -n '52,92p' crates/admin-cli/src/expected_machines/patch/args.rs

echo "== generated add page =="
sed -n '1,180p' docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md

echo "== generated patch page =="
sed -n '1,180p' docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md

echo "== documentation generation references =="
rg -n -i 'bookworm|pandoc|clap_mangen|expected-machine-(add|patch)|nico-admin-cli' \
  Makefile* justfile* .github scripts docs crates 2>/dev/null | head -250

Repository: NVIDIA/infra-controller

Length of output: 48024


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "== documentation generation task =="
sed -n '100,132p' Makefile.toml

echo "== CLI documentation generator references =="
rg -n -C3 'generate-cli-docs|after_long_help|long_help|Examples|examples' crates/admin-cli/src crates/admin-cli/Cargo.toml

echo "== tracked documentation status =="
git status --short -- \
  crates/admin-cli/src/expected_machines/add/args.rs \
  crates/admin-cli/src/expected_machines/patch/args.rs \
  docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-add.md \
  docs/manuals/nico-admin-cli/commands/expected-machine/expected-machine-patch.md

Repository: NVIDIA/infra-controller

Length of output: 50382


Regenerate the expected-machine reference pages.

The Rust help blocks contain eight add examples and seven patch examples, but the generated pages contain only four examples each. Run the repository’s gen-cli-docs task with the Bookworm/Pandoc toolchain and commit both updated pages.

🤖 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/admin-cli/src/expected_machines/add/args.rs` around lines 154 - 162,
Regenerate the expected-machine reference pages using the repository’s
gen-cli-docs task with the Bookworm/Pandoc toolchain, ensuring all eight add
examples and seven patch examples are included. Commit the resulting updates to
both generated reference pages without modifying the Rust help definitions.

Sources: Path instructions, Learnings

Keep Expected Machine help grammatical in both the live CLI and generated reference pages.

Rephrase wording that loses apostrophes during the roff-to-Markdown conversion, and distinguish initial pause and DPF behavior between add and patch.

This supports NVIDIA#4894

Signed-off-by: Chet Nichols III <chetn@nvidia.com>
@github-actions

Copy link
Copy Markdown

@chet
chet merged commit dcd90af into NVIDIA:main Aug 13, 2026
66 checks passed
@chet
chet deleted the gh-issue-4894 branch August 13, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants