🌱 Bump KAL & add nonullable and forbiddenmarkers linter #12724
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
do-not-merge/needs-area
Signed-off-by: Stefan Büringer [email protected]
sbueringer
force-pushed
the
pr-kal-nullable
branch
from
581f1e7 to
2604e41
Compare
k8s-ci-robot
added
size/XS
| - "conditions" # Ensure conditions have the correct json tags and markers. | ||
| - "conflictingmarkers" | ||
| - "duplicatemarkers" # Ensure there are no exact duplicate markers. for types and fields. | ||
| #- "forbiddenmarkers" # Ensure that types and fields do not contain any markers that are forbidden. |
There was a problem hiding this comment.
I definitely want to surface the linter here to make folks aware that this exists
@JoelSpeed Do you have any suggestions about markers that we should forbid?
There was a problem hiding this comment.
This new rule is actually a basis for other rules to be implemented, e.g. nonullable (Already done) which prevents folks from using +nullable is just a static config to this linter.
At the moment, I think this is really for folks who have custom or specific reasons to prevent the usage of a specific marker.
Scanning through the Kubebuilder markers, its possible as a project that we might decide we don't want anything that is +kubebuilder:validation:Schemaless or uses +kubebuilder:validation:UniqueItems (IIRC this doesn't actually work)
There was a problem hiding this comment.
We need Schemaless in a few cases, but I think we'll have discussions anyway if we want to add this to any new fields. I'll add a rule for UniqueItems.
EDIT: Or maybe I'll just leave it as is for now. I have no plans to start using UniqueItems but if I want to forbid it I would probably want to link to an issue that mentions that it doesn't work (but probably not worth the effort to look this up)
|
/area ci |
k8s-ci-robot
added
area/ci
|
/assign @JoelSpeed @sivchari |
|
/approve |
k8s-ci-robot
added
the
approved
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: fabriziopandini, JoelSpeed The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm Thx! |
k8s-ci-robot
added
the
lgtm
|
LGTM label has been added. Git tree hash: a72d6df930194dc5789264d5a1344f1f1951da79
|
5 participants
Signed-off-by: Stefan Büringer [email protected]
What this PR does / why we need it:
Which issue(s) this PR fixes (optional, in
fixes #<issue number>(, fixes #<issue_number>, ...)format, will close the issue(s) when PR gets merged):Fixes #