CNTRLPLANE-3734: Add docs for Agentic SDLC, architecture and contributing - #945
CNTRLPLANE-3734: Add docs for Agentic SDLC, architecture and contributing#945tchap wants to merge 2 commits into
Conversation
|
@tchap: This pull request references CNTRLPLANE-3734 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target the "5.0.0" version, but no target version was set. DetailsIn response to this:
Instructions 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 openshift-eng/jira-lifecycle-plugin repository. |
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
WalkthroughAdds documentation covering operator architecture, agent and contributor guidance, testing workflows, and README navigation. ChangesRepository documentation
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 15✅ Passed checks (15 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ARCHITECTURE.md (1)
1-155: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winRename this file to lowercase.
ARCHITECTURE.mdviolates the repo rule that all filenames must be lowercase. As per coding guidelines,All filenames should be lowercase.🤖 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 `@ARCHITECTURE.md` around lines 1 - 155, Rename the ARCHITECTURE.md document to use a lowercase filename to match the repository naming rule. Update the documentation file itself and any references that point to it so the lowercase name is used consistently, ensuring the architecture content remains unchanged while the identifier complies with the repo-wide lowercase filename convention.Source: Coding guidelines
🤖 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 `@CONTRIBUTING.md`:
- Around line 69-71: The new documentation filenames violate the lowercase
filename rule in CONTRIBUTING.md, so update the docs naming to comply or
explicitly document an exception for instruction files. Make the fix in the
places that introduce AGENTS.md, CLAUDE.md, and CONTRIBUTING.md, and ensure the
guidance in the repository’s docs policy clearly states whether standard
instruction docs are exempt from the lowercase requirement.
---
Outside diff comments:
In `@ARCHITECTURE.md`:
- Around line 1-155: Rename the ARCHITECTURE.md document to use a lowercase
filename to match the repository naming rule. Update the documentation file
itself and any references that point to it so the lowercase name is used
consistently, ensuring the architecture content remains unchanged while the
identifier complies with the repo-wide lowercase filename convention.
🪄 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: ddf468f3-cc1e-4427-bb71-667c75d375c8
📒 Files selected for processing (5)
AGENTS.mdARCHITECTURE.mdCLAUDE.mdCONTRIBUTING.mdREADME.md
There was a problem hiding this comment.
♻️ Duplicate comments (1)
CONTRIBUTING.md (1)
69-71: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep the docs filename policy consistent.
CONTRIBUTING.mdstill conflicts with the repo rule that all filenames should be lowercase. If standard instruction docs are meant to be exempt, state that explicitly here; otherwise rename them to match the policy.🤖 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 `@CONTRIBUTING.md` around lines 69 - 71, The filename policy in CONTRIBUTING.md conflicts with the stated lowercase-only rule, so update the docs guidance to be consistent. Either explicitly add an exception for standard documentation filenames like CONTRIBUTING.md in the filename policy, or change the policy text so it applies uniformly and no longer contradicts existing docs file names; use the CONTRIBUTING.md section that lists filename conventions as the place to make this clarification.Source: Coding guidelines
🤖 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.
Duplicate comments:
In `@CONTRIBUTING.md`:
- Around line 69-71: The filename policy in CONTRIBUTING.md conflicts with the
stated lowercase-only rule, so update the docs guidance to be consistent. Either
explicitly add an exception for standard documentation filenames like
CONTRIBUTING.md in the filename policy, or change the policy text so it applies
uniformly and no longer contradicts existing docs file names; use the
CONTRIBUTING.md section that lists filename conventions as the place to make
this clarification.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2f3b604c-442e-40f2-a452-a87f21a5c8ff
📒 Files selected for processing (5)
AGENTS.mdARCHITECTURE.mdCLAUDE.mdCONTRIBUTING.mdREADME.md
✅ Files skipped from review due to trivial changes (3)
- CLAUDE.md
- AGENTS.md
- ARCHITECTURE.md
🚧 Files skipped from review as they are similar to previous changes (1)
- README.md
Add AGENTS.md, ARCHITECTURE.md, CLAUDE.md, and CONTRIBUTING.md with verified codebase documentation. Move OTE test instructions from README.md into CONTRIBUTING.md to avoid duplication.
|
/retitle CNTRLPLANE-3734: Add docs for Agentic SDLC, architecture and contributing |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
README.md (1)
65-85: 🩺 Stability & Availability | 🔵 TrivialDocument the cleanup step for CVO overrides.
These commands leave the operator Deployment unmanaged by CVO. Add the inverse operation to remove the override and restore normal CVO reconciliation after testing; otherwise developers can unintentionally leave upgrade management disabled.
🤖 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 `@README.md` around lines 65 - 85, Update the custom operator image instructions after the deployment patch to document cleanup: remove the kube-controller-manager-operator entry from spec.overrides using the appropriate ClusterVersion patch, restoring CVO management and normal reconciliation after testing.
🤖 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 `@AGENTS.md`:
- Line 84: Update the testing guidance in AGENTS.md to require unit tests for
behavior or implementation changes, with E2E tests for significant features.
Explicitly state the expected validation for documentation-only changes, without
requiring unit tests for them.
In `@CONTRIBUTING.md`:
- Line 91: Update the documentation link in the unit-testing guidance in
CONTRIBUTING.md from the internal cmd/go test package URL to the public cmd/go
documentation URL, leaving the surrounding testing instructions unchanged.
In `@README.md`:
- Around line 87-103: Remove the duplicated OTE command instructions from the
README Tests section and replace them with a concise reference directing
contributors to the canonical OTE guidance in CONTRIBUTING.md. Ensure only one
location remains authoritative for these test commands.
---
Nitpick comments:
In `@README.md`:
- Around line 65-85: Update the custom operator image instructions after the
deployment patch to document cleanup: remove the
kube-controller-manager-operator entry from spec.overrides using the appropriate
ClusterVersion patch, restoring CVO management and normal reconciliation after
testing.
🪄 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: 4536a6d5-3ff7-4c8e-b37a-89d3ed7a69d2
📒 Files selected for processing (5)
AGENTS.mdARCHITECTURE.mdCLAUDE.mdCONTRIBUTING.mdREADME.md
…penshift#944 - AGENTS.md: add Critical Rules and What NOT to Do sections - ARCHITECTURE.md: add Design Decisions section (8 items) - README.md: streamline with Quick Start, concise sections, doc links
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
README.md (1)
65-78: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDocument how to restore CVO management after debugging.
This procedure sets the operator deployment to
unmanaged: true, but never instructs contributors to remove that override afterward. Users can leave the operator permanently outside CVO reconciliation, causing drift or upgrade/recovery issues. Add cleanup instructions that remove only this override while preserving any unrelated overrides.🤖 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 `@README.md` around lines 65 - 78, Extend the custom operator image procedure after the override command with cleanup instructions for restoring CVO management. Document removing only the matching kube-controller-manager-operator override, preserving any unrelated entries in spec.overrides, and show the command or established mechanism contributors should use to perform that targeted removal.
🤖 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.
Outside diff comments:
In `@README.md`:
- Around line 65-78: Extend the custom operator image procedure after the
override command with cleanup instructions for restoring CVO management.
Document removing only the matching kube-controller-manager-operator override,
preserving any unrelated entries in spec.overrides, and show the command or
established mechanism contributors should use to perform that targeted removal.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 56a66483-11ea-40b0-864a-056062fa57d1
📒 Files selected for processing (3)
AGENTS.mdARCHITECTURE.mdREADME.md
🚧 Files skipped from review as they are similar to previous changes (2)
- AGENTS.md
- ARCHITECTURE.md
|
Closing since there is #944 already. |
Add AGENTS.md, ARCHITECTURE.md, CLAUDE.md, and CONTRIBUTING.md with verified codebase documentation. Move OTE test instructions from README.md into CONTRIBUTING.md to avoid duplication.
Created as a draft to prevent unnecessary CI runs until accepted.
Summary by CodeRabbit