ROSAENG-62596: fix: set cluster as non namespaced for client generator - #285
Conversation
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
openshift-ci
Bot
removed
the
do-not-merge/work-in-progress
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: openshift-online/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughThe Cluster resource is marked non-namespaced. Client generation and wrappers now expose ChangesCluster client access
Estimated code review effort: 2 (Simple) | ~10 minutes Sequence Diagram(s)sequenceDiagram
participant ClusterResource
participant WireGenerator
participant ClusterClient
ClusterResource->>WireGenerator: provide nonNamespaced marker
WireGenerator->>ClusterClient: generate Clusters() accessor
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 10 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (10 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/e2e-sdk/sdk_sanity_test.go (1)
671-671: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the obsolete
customerAccountIDparameter.
deleteClusternow uses the platform-scoped client at Line [671] and does not readcustomerAccountID. Remove the parameter and update both callers. This keeps the helper contract aligned with the non-namespaced API.🤖 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 `@test/e2e-sdk/sdk_sanity_test.go` at line 671, Remove the unused customerAccountID parameter from deleteCluster, then update both deleteCluster call sites to use its revised signature. Keep the platform-scoped Clusters client flow 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 `@hack/clientset/cmd/wire-gen/main.go`:
- Around line 209-214: Preserve the existing public Clusters(namespace string)
API across all generated layers. In hack/clientset/cmd/wire-gen/main.go lines
209-214, emit the namespaced-compatible interface signature; in lines 230-238,
make the generated implementation use that same signature while delegating to
w.inner.Clusters(). Apply the corresponding interface and implementation
signature updates in clientset/wrappers/wire_wrappers_generated.go lines 235 and
252-253.
---
Nitpick comments:
In `@test/e2e-sdk/sdk_sanity_test.go`:
- Line 671: Remove the unused customerAccountID parameter from deleteCluster,
then update both deleteCluster call sites to use its revised signature. Keep the
platform-scoped Clusters client flow unchanged.
🪄 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-online/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: ae72e998-ad25-48bd-ab68-4dfe4fc9be1e
⛔ Files ignored due to path filters (4)
clientset/generated/typed/v1alpha1/internalversion/cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_cluster.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/fake/fake_v1alpha1_client.gois excluded by!**/generated/**clientset/generated/typed/v1alpha1/internalversion/v1alpha1_client.gois excluded by!**/generated/**
📒 Files selected for processing (5)
clientset/hyperfleet.goclientset/wrappers/wire_wrappers_generated.gohack/clientset/cmd/wire-gen/main.gohyperfleet-operator/api/v1alpha1/cluster_types.gotest/e2e-sdk/sdk_sanity_test.go
857222e to
aa84ff0
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. |
aa84ff0 to
c6cf933
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. |
cdoan1
changed the title
|
@gdbranco: This pull request references ROSAENG-62596 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. |
openshift-ci-robot
added
the
jira/valid-reference
c6cf933 to
568b9ed
Compare
|
/approve |
openshift-ci
Bot
added
the
approved
|
/test on-demand-e2e |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cdoan1, gdbranco 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 |
a1bbdf1
into
openshift-online:main
3 participants
Description
Type of Change
Testing
make test)Checklist
Summary by CodeRabbit
New Features
Bug Fixes