Improve documentation for BIOS BMC CRDs#761
Open
nagadeesh-nagaraja wants to merge 1 commit intomainfrom
Open
Conversation
github-actions
bot
added
documentation
Contributor
WalkthroughUpdated docs navigation and added two new pages. Rewrote and standardized BIOS/BMC concept pages (spec, status, state machines, workflows, troubleshooting, examples). Added new BMCSettingsSet concept doc and a CRD/controller documentation checklist page. Changes
Sequence Diagram(s)(omitted — changes are documentation-only and do not introduce executable control-flow changes) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
docs/concepts/bmcsettingsset.md (1)
44-44: Minor grammar fix: add explicit subject.The validation rule description is missing an explicit subject for clarity.
📝 Proposed grammar fix
-- `variables` can only be used with `format`. +- `variables` can only be used when `format` is set.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/concepts/bmcsettingsset.md` at line 44, Update the sentence to include an explicit subject for clarity: change the line "`variables` can only be used with `format`." to something like "The `variables` option can only be used with the `format` option." so it clearly reads that the variables option is restricted to use with the format option (referencing the `variables` and `format` tokens in the documentation).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/concepts/biossettings.md`:
- Line 19: The table incorrectly marks `spec.serverRef.name` as required; update
the docs to mark it as "No" and clarify that `BIOSSettingsSpec.ServerRef` is
optional in the schema (it uses `omitempty` and has no `+required`), and add a
short note after the table describing any runtime expectations (e.g.,
controllers may require a serverRef in certain operations) rather than claiming
compile-time requiredness; refer to `spec.serverRef.name` and the
`BIOSSettingsSpec.ServerRef` type when making these changes.
In `@docs/concepts/biosversion.md`:
- Line 19: The table incorrectly marks `spec.serverRef.name` as required; update
the docs to show "No" for requiredness to match the API type
`BIOSVersionSpec.ServerRef` which is `+optional`/`omitempty`, and retain any
controller-level expectation as explanatory text rather than as a required flag;
ensure references to `spec.serverRef`, `spec.serverRef.name`, and
`BIOSVersionSpec.ServerRef` are consistent with the API schema.
In `@docs/concepts/bmcsettings.md`:
- Line 33: The table row describing spec.serverMaintenanceRefs[] currently
appears under the "Status Fields In Detail" section which is misleading; either
move the row entry for spec.serverMaintenanceRefs[] into the Spec section (so it
lives with other spec fields) or rename "Status Fields In Detail" to something
like "Spec and Status Fields" and update the row description to clarify it is a
spec field used for diagnostics; update the table header and any surrounding
text to reflect the chosen change and ensure the symbol
spec.serverMaintenanceRefs[] is referenced consistently.
In `@docs/concepts/bmcversion.md`:
- Line 19: Update the table entry for spec.bmcRef.name to reflect schema-level
optionality: change the “Required” column from Yes to No (or add a parenthetical
like “No (controller may require at runtime)”), because the API type defines
BMCRef as optional (omitempty and no +required marker). Reference the BMCRef
type and the spec.bmcRef.name field in the docs so readers know the difference
between schema-level requiredness and controller-level runtime requirements.
---
Nitpick comments:
In `@docs/concepts/bmcsettingsset.md`:
- Line 44: Update the sentence to include an explicit subject for clarity:
change the line "`variables` can only be used with `format`." to something like
"The `variables` option can only be used with the `format` option." so it
clearly reads that the variables option is restricted to use with the format
option (referencing the `variables` and `format` tokens in the documentation).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7f891e00-f7fe-4ce5-aecd-024587f130c3
📒 Files selected for processing (11)
docs/.vitepress/config.mtsdocs/concepts/biossettings.mddocs/concepts/biossettingsset.mddocs/concepts/biosversion.mddocs/concepts/biosversionset.mddocs/concepts/bmcsettings.mddocs/concepts/bmcsettingsset.mddocs/concepts/bmcversion.mddocs/concepts/bmcversionset.mddocs/concepts/index.mddocs/development/crd_controller_docs.md
nagadeesh-nagaraja
force-pushed
the
doc-update
branch
from
004e557 to
c6c8afb
Compare
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/concepts/bmcsettings.md (1)
79-79: Optional: Consider hyphenating "policy driven" as a compound modifier.Line 79: "Request maintenance per server (policy driven)" could be written as "policy-driven" when used as a compound adjective before a noun. However, the current usage in parentheses is acceptable.
📝 Optional style improvement
- - Request maintenance per server (policy driven) and wait for approval. + - Request maintenance per server (policy-driven) and wait for approval.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/concepts/bmcsettings.md` at line 79, Update the phrase "Request maintenance per server (policy driven)" to use the compound adjective form "policy-driven" so it reads "Request maintenance per server (policy-driven)"; locate the text containing the exact phrase "Request maintenance per server (policy driven)" in the docs/concepts/bmcsettings.md content and replace it accordingly to maintain consistent style.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@docs/concepts/bmcsettings.md`:
- Line 79: Update the phrase "Request maintenance per server (policy driven)" to
use the compound adjective form "policy-driven" so it reads "Request maintenance
per server (policy-driven)"; locate the text containing the exact phrase
"Request maintenance per server (policy driven)" in the
docs/concepts/bmcsettings.md content and replace it accordingly to maintain
consistent style.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8d6e6670-6500-40ab-8cc8-a38c01fb9c88
📒 Files selected for processing (11)
docs/.vitepress/config.mtsdocs/concepts/biossettings.mddocs/concepts/biossettingsset.mddocs/concepts/biosversion.mddocs/concepts/biosversionset.mddocs/concepts/bmcsettings.mddocs/concepts/bmcsettingsset.mddocs/concepts/bmcversion.mddocs/concepts/bmcversionset.mddocs/concepts/index.mddocs/development/crd_controller_docs.md
✅ Files skipped from review due to trivial changes (5)
- docs/concepts/index.md
- docs/.vitepress/config.mts
- docs/concepts/biosversion.md
- docs/concepts/biossettings.md
- docs/concepts/bmcversionset.md
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/concepts/biosversionset.md
nagadeesh-nagaraja
changed the title
xkonni
approved these changes
Mar 31, 2026
Contributor
xkonni
left a comment
xkonni
left a comment
There was a problem hiding this comment.
minor questions, high quality overall.
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
3 participants
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.
Proposed Changes
Update documentation
Summary by CodeRabbit