fix: some typo and wrong kubebuilder comments#244
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
There was a problem hiding this comment.
Pull request overview
Fixes API/CRD typos and adjusts kubebuilder-generated schema defaults so the RoleBasedGroup/RoleBasedGroupSet workload reference defaults align with v1alpha2 RoleInstanceSet.
Changes:
- Update RoleBasedGroup/RoleBasedGroupSet CRD schema defaults to
workloads.x-k8s.io/v1alpha2+RoleInstanceSet. - Fix several CRD/API doc typos (spacing/wording).
- Add missing
v1alpha1Go types forRoleBasedGroupScalingAdapter(to match existing CRD versions).
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| config/crd/bases/workloads.x-k8s.io_rolebasedgroupsets.yaml | Update workload apiVersion/kind defaults in schema |
| config/crd/bases/workloads.x-k8s.io_rolebasedgroups.yaml | Update workload apiVersion/kind defaults in schema |
| config/crd/bases/workloads.x-k8s.io_instances.yaml | Fix CRD schema description typos/wording |
| api/workloads/v1alpha2/rolebasedgroup_types.go | Update kubebuilder default markers for WorkloadSpec |
| api/workloads/v1alpha2/coordinatedpolicy_types.go | Comment wording fix |
| api/workloads/v1alpha1/rolebasedgroupscalingadapter_types.go | Add v1alpha1 ScalingAdapter API types |
| api/workloads/v1alpha1/instance_types.go | Fix comment typos |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The underlying workload will use InstanceSet. | ||
| properties: | ||
| apiVersion: | ||
| default: workloads.x-k8s.io/v1alpha1 | ||
| default: workloads.x-k8s.io/v1alpha2 | ||
| pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/v[0-9]+((alpha|beta)[0-9]+)?$ | ||
| type: string | ||
| kind: | ||
| default: InstanceSet | ||
| default: RoleInstanceSet |
There was a problem hiding this comment.
The description still says the underlying workload will use "InstanceSet", but the defaults in this schema block are now apiVersion=v1alpha2 and kind=RoleInstanceSet. Please update the description to match the actual default/behavior (e.g., RoleInstanceSet), otherwise the CRD docs will be misleading.
| The underlying workload will use InstanceSet. | ||
| properties: | ||
| apiVersion: | ||
| default: workloads.x-k8s.io/v1alpha1 | ||
| default: workloads.x-k8s.io/v1alpha2 | ||
| pattern: ^[a-z0-9]([-a-z0-9]*[a-z0-9])?(\.[a-z0-9]([-a-z0-9]*[a-z0-9])?)*/v[0-9]+((alpha|beta)[0-9]+)?$ | ||
| type: string | ||
| kind: | ||
| default: InstanceSet | ||
| default: RoleInstanceSet |
There was a problem hiding this comment.
The description still claims the underlying workload uses "InstanceSet", but the defaults immediately below are apiVersion=v1alpha2 and kind=RoleInstanceSet. Please align the description with the default/actual workload type to avoid confusing users reading the CRD schema.
Ⅰ. Motivation
Ⅱ. Modifications
Ⅲ. Does this pull request fix one issue?
fixes #XXXX
Ⅳ. List the added test cases (unit test/integration test) if any, please explain if no tests are needed.
Ⅴ. Describe how to verify it
VI. Special notes for reviews
Checklist
make fmt.