Add user and developer documentation - #67
Conversation
WalkthroughThe README is condensed into a shorter entry point. New documentation covers the API, architecture, development workflow, and installation through OLM or without OLM. ChangesDocumentation overhaul
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jcpowermac The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/user/install-with-olm.md (1)
131-131: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRemove the stray trailing line artifact.
Line 131 contains a standalone
131, which renders as unintended content.🤖 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 `@docs/user/install-with-olm.md` at line 131, Remove the stray standalone artifact from the install-with-olm docs so the rendered page no longer includes unintended content. Locate the markdown section around the installation steps in the document and delete the extra trailing `131` line, leaving the surrounding prose unchanged.
🤖 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 `@docs/dev/api.md`:
- Around line 75-78: The fenced code block in the table example is missing a
language identifier, causing the MD040 warning. Update the example under the
docs section to use a text-labeled fence for the sample output, keeping the
content unchanged while ensuring the fenced block is explicitly marked in the
markdown.
In `@docs/dev/architecture.md`:
- Around line 5-27: The fenced code block in the architecture documentation is
missing a language tag, which violates MD040. Update the fenced block in the
docs/dev/architecture.md content so each fence explicitly declares a language
such as text, and apply the same fix to the other fenced block referenced in the
comment.
- Around line 90-98: The dependency table in docs/dev/architecture.md has
truncated OpenShift version entries; update the openshift/api and
openshift/client-go rows in the dependency list to use the full pseudo-versions
from go.mod, keeping the rest of the table unchanged. Use the existing
dependency table entries to locate the two OpenShift rows and replace only their
Version values with the exact full versions.
In `@docs/user/install-with-olm.md`:
- Around line 38-51: Add the missing prerequisites before the Subscription in
the install guide: create the target Namespace and an OperatorGroup for
openshift-vcf-migration, then keep the Subscription pointing at that namespace.
Update the numbered steps in install-with-olm and place the new Namespace and
OperatorGroup YAML immediately before the existing Subscription example so the
manual install path works. Use the Subscription, OperatorGroup, and
openshift-vcf-migration symbols to keep the placement clear.
In `@docs/user/install-without-olm.md`:
- Line 75: The migration instructions mention setting spec.state to Running but
do not give a concrete way to do it, so update the install-without-olm doc
section that describes starting the migration to include an explicit oc patch
command for the vcfm resource (for example, the vcf-migration instance in the
openshift-vcf-migration namespace) so users can actually trigger the state
change. Keep the surrounding explanation about operator phases, and place the
command immediately after the sentence about setting spec.state to Running.
---
Outside diff comments:
In `@docs/user/install-with-olm.md`:
- Line 131: Remove the stray standalone artifact from the install-with-olm docs
so the rendered page no longer includes unintended content. Locate the markdown
section around the installation steps in the document and delete the extra
trailing `131` line, leaving the surrounding prose unchanged.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 3504bcc6-9d8c-4150-b257-c2b508411a6a
📒 Files selected for processing (6)
README.mddocs/dev/api.mddocs/dev/architecture.mddocs/dev/development.mddocs/user/install-with-olm.mddocs/user/install-without-olm.md
8d2aa87 to
ef1a7d4
Compare
User docs cover installation with and without OLM. Developer docs cover the API reference, architecture/reconciliation flow, and the build/test/deploy workflow. README simplified to link both. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
ef1a7d4 to
e6c3014
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
@jcpowermac: all tests passed! Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
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 `@docs/dev/api.md`:
- Line 34: Update the VSpherePlatformFailureDomainSpec heading to clarify that
it comes from the OpenShift API configv1, formatting configv1 as code and
identifying openshift/api as the repository path if retained.
In `@docs/user/install-without-olm.md`:
- Around line 52-53: Update the installation guide’s example resource name and
every related reference from vcf-migration to cluster, including the patch and
describe commands, so they target the reconciler’s singleton resource.
🪄 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: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 4c708321-e6b6-4afc-a95b-5e8b21b3764e
📒 Files selected for processing (6)
README.mddocs/dev/api.mddocs/dev/architecture.mddocs/dev/development.mddocs/user/install-with-olm.mddocs/user/install-without-olm.md
🚧 Files skipped from review as they are similar to previous changes (4)
- docs/user/install-with-olm.md
- docs/dev/development.md
- docs/dev/architecture.md
- README.md
|
|
||
| The secret must contain keys in the format `<vcenter-fqdn>.username` and `<vcenter-fqdn>.password`. | ||
|
|
||
| ### VSpherePlatformFailureDomainSpec (from openshift/api configv1) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- target lines ---'
sed -n '25,42p' docs/dev/api.md
printf '%s\n' '--- source-reference occurrences ---'
rg -n -C 2 'VSpherePlatformFailureDomainSpec|openshift/api|configv1' docs/dev/api.md
printf '%s\n' '--- repository status ---'
git status --shortRepository: openshift/vcf-migration-operator
Length of output: 1764
🏁 Script executed:
#!/bin/bash
set -eu
if [ -f docs/dev/api.md ]; then
nl -ba docs/dev/api.md | sed -n '28,38p'
else
printf '%s\n' 'docs/dev/api.md not found'
exit 1
fiRepository: openshift/vcf-migration-operator
Length of output: 206
🏁 Script executed:
#!/bin/bash
set -eu
python3 - <<'PY'
from pathlib import Path
path = Path("docs/dev/api.md")
text = path.read_text(encoding="utf-8")
lines = text.splitlines()
for number in range(28, 39):
if number <= len(lines):
print(f"{number}: {lines[number - 1]}")
target = "### VSpherePlatformFailureDomainSpec (from openshift/api configv1)"
print(f"exact_target_present={target in text}")
print(f"target_count={text.count(target)}")
PYRepository: openshift/vcf-migration-operator
Length of output: 749
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- OpenShift API imports and type references ---'
rg -n -C 2 'openshift/api|configv1|VSpherePlatformFailureDomainSpec' \
--glob '!docs/dev/api.md' --glob '!vendor/**' .
printf '%s\n' '--- module metadata ---'
rg -n -C 2 'github.com/openshift/api|module ' go.mod go.sum 2>/dev/null || trueRepository: openshift/vcf-migration-operator
Length of output: 50388
Clarify the OpenShift API source reference.
Use from the OpenShift API \configv1`. If the repository path is required, format openshift/api` as code and identify it as the repository path.
🧰 Tools
🪛 LanguageTool
[grammar] ~34-~34: Ensure spelling is correct
Context: ...rmFailureDomainSpec (from openshift/api configv1) | Field | Type | Required | Description ...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 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 `@docs/dev/api.md` at line 34, Update the VSpherePlatformFailureDomainSpec
heading to clarify that it comes from the OpenShift API configv1, formatting
configv1 as code and identifying openshift/api as the repository path if
retained.
Source: Linters/SAST tools
| name: vcf-migration | ||
| namespace: openshift-vcf-migration |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the reconciler’s singleton name in the example.
api/v1alpha1/vmwarecloudfoundationmigration_types.go defines SingletonName = "cluster", and internal/controller/vmwarecloudfoundationmigration_controller.go ignores resources with any other name. This guide creates vcf-migration, then patches and describes that ignored object. Users will not start or observe the migration.
Change all references to cluster.
Suggested documentation patch
metadata:
- name: vcf-migration
+ name: cluster
...
-oc patch vcfm vcf-migration -n openshift-vcf-migration --type merge -p '{"spec":{"state":"Running"}}'
+oc patch vcfm cluster -n openshift-vcf-migration --type merge -p '{"spec":{"state":"Running"}}'
...
-oc describe vcfm vcf-migration -n openshift-vcf-migration
+oc describe vcfm cluster -n openshift-vcf-migrationAlso applies to: 78-78, 94-94
🤖 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 `@docs/user/install-without-olm.md` around lines 52 - 53, Update the
installation guide’s example resource name and every related reference from
vcf-migration to cluster, including the patch and describe commands, so they
target the reconciler’s singleton resource.
User docs cover installation with and without OLM. Developer docs cover the API reference, architecture/reconciliation flow, and the build/test/deploy workflow. README simplified to link both.
Summary by CodeRabbit