Add Kubernetes API Linter to kueue-operator#1282
Add Kubernetes API Linter to kueue-operator#1282kannon92 wants to merge 1 commit intoopenshift:mainfrom
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: kannon92 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 |
openshift-ci
bot
added
the
approved
kannon92
force-pushed
the
add-kal-linter
branch
4 times, most recently
from
44390a0 to
4a400db
Compare
| // spec holds user settable values for configuration | ||
| // +required | ||
| Spec KueueOperandSpec `json:"spec"` | ||
| Spec KueueOperandSpec `json:"spec,omitzero"` |
There was a problem hiding this comment.
Is there any concerns on breaking changes for omitzero or omitempty?
I don't have any CRD changes but I guess its worth asking if there could be any UX issues with adding these tags.
There was a problem hiding this comment.
I think this is going to be safe. Looking through, I can see that the empty marshalled spec object would have been rejected by the fact you have at least 1 required field that doesn't permit its empty value in the spec.
So previously a user who set nothing, would have marshalled the empty object, and got a rejection. Now, they wouldn't marshal anything, and would still get a rejection
| - ["default", "kubebuilder:default"] | ||
| - ["required", "kubebuilder:validation:Required", "k8s:required"] | ||
| description: "A field with a default value cannot be required" | ||
| conditions: |
There was a problem hiding this comment.
I noticed a couple of differences from upstream, is this expected?
https://github.com/kubernetes-sigs/kueue/blob/main/.golangci-kal.yml#L50-L61
There was a problem hiding this comment.
Pushed up a change for conditions so it matches.
|
FWIW, the make targets worked for me. I will defer to others with comments. |
kannon92
force-pushed
the
add-kal-linter
branch
from
05f6a8e to
a5565ad
Compare
kannon92
force-pushed
the
add-kal-linter
branch
from
a5565ad to
0b54310
Compare
|
@kannon92: 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. |
4 participants
This should help enforce standards on new API changes similar to openshift/api.
This is not yet included in CI but we can at least allow people to run it on some PRs like #1266 or #1213.