docs(nvcf-cli): align skill reference docs with the CLI code#300
docs(nvcf-cli): align skill reference docs with the CLI code#300mesutoezdil wants to merge 1 commit into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
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 ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
📝 WalkthroughWalkthroughThe CLI skill references and configuration template are updated to match current command syntax, supported flags, defaults, exit codes, and the ChangesCLI reference alignment
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@ai-tooling/user/skills/nvcf-self-managed-cli/reference/flags.md`:
- Around line 108-113: Update the --max-request-concurrency row in the CLI flags
reference to document the default as 0 (server default) instead of unset (server
default), matching the implementation and stated CLI behavior.
- Around line 42-56: Update the cluster register command syntax in commands.md
to omit the region argument or mark it optional, matching the documented
us-west-1 default in the flags table; keep --name and --nca-id required.
🪄 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: Enterprise
Run ID: bfa078f5-7efb-4aae-afa1-4d10c72af072
⛔ Files ignored due to path filters (1)
src/clis/nvcf-cli/internal/agentskill/skilldata_generated.gois excluded by!**/*_generated.go
📒 Files selected for processing (4)
ai-tooling/user/skills/nvcf-self-managed-cli/reference/commands.mdai-tooling/user/skills/nvcf-self-managed-cli/reference/exit-codes.mdai-tooling/user/skills/nvcf-self-managed-cli/reference/flags.mdsrc/clis/nvcf-cli/.nvcf-cli.yaml.template
1284150 to
b77dbce
Compare
b77dbce to
181640b
Compare
There was a problem hiding this comment.
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 `@ai-tooling/user/skills/nvcf-self-managed-cli/reference/flags.md`:
- Line 20: Update the --no-apply flag description in flags.md to remove the
repeated “only,” changing “install only - emit YAML only, don't kubectl apply”
to clearer wording equivalent to “install only - emit YAML, do not apply.”
🪄 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: Enterprise
Run ID: 5d3290f6-c6e6-42c3-9b74-abdcbd2b122a
⛔ Files ignored due to path filters (1)
src/clis/nvcf-cli/internal/agentskill/skilldata_generated.gois excluded by!**/*_generated.go
📒 Files selected for processing (4)
ai-tooling/user/skills/nvcf-self-managed-cli/reference/commands.mdai-tooling/user/skills/nvcf-self-managed-cli/reference/exit-codes.mdai-tooling/user/skills/nvcf-self-managed-cli/reference/flags.mdsrc/clis/nvcf-cli/.nvcf-cli.yaml.template
🚧 Files skipped from review as they are similar to previous changes (2)
- src/clis/nvcf-cli/.nvcf-cli.yaml.template
- ai-tooling/user/skills/nvcf-self-managed-cli/reference/commands.md
84d68fc to
c082868
Compare
|
Please rebase then good to merge assuming CI passes ty |
Remove flags and commands that do not exist (--identity-source, --show-events, cluster get, api-key --id), fix wrong defaults (--region is optional with default us-west-1, --max-request-concurrency is unset by default), drop exit code 4 which no code path returns, mark --drain-active as a plain bool, and correct the state file path to ~/.nvcf-cli.state. Replace em dashes with plain dashes per the ASCII-only style rule. Regenerate skilldata_generated.go. Closes NVIDIA#299 Signed-off-by: mesutoezdil <mesudozdil@gmail.com>
c082868 to
51f2377
Compare
done |
TL;DR
The nvcf-self-managed-cli reference docs described flags, commands, defaults, and exit codes that do not exist in the CLI: --identity-source, --show-events, cluster get, api-key --id, a required --region, a max-request-concurrency default of 10, exit code 4, and a wrong state file path. All corrected against src/clis/nvcf-cli and skilldata_generated.go regenerated. go build and the agentskill and cmd tests pass.
Issues
Closes #299
Summary by CodeRabbit
nvcf-clicommand reference for self-hosted lifecycle draining to use a presence-style--drain-activeflag.--regionoptional for cluster registration.KEY-IDarguments.