-
Notifications
You must be signed in to change notification settings - Fork 410
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add x-kubernetes-{embedded-resource,preserve-unknown-fields} support #345
Add x-kubernetes-{embedded-resource,preserve-unknown-fields} support #345
Conversation
07d2cc6
to
a066658
Compare
a066658
to
a3cca5d
Compare
Fixes #278 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
/assign @DirectXMan12
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nits / docs typos inline, otherwise LGTM
17d8a01
to
dad3ed3
Compare
dad3ed3
to
41fb213
Compare
41fb213
to
b20b951
Compare
b20b951
to
8a4bf07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/lgtm
@DirectXMan12 ptal |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tiny nit inline
(also, please run go mod tidy
)
pkg/crd/markers/validation.go
Outdated
@@ -175,6 +182,26 @@ type Default struct { | |||
Value interface{} | |||
} | |||
|
|||
// +controllertools:marker:generateHelp:category="CRD pruning" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: can you stick this in the validation category? it just makes it less weird to show in the docs (or we should come up with a general category for non-validation schema things, along with listType
and such). cc @apelisse
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've used "CRD topology", wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(or "CRD processing" or "CRD structure" -- whatever you think gets the point across)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought about structure too. processing is super "imperative", it's such a bad word ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CRD processing
fits best. It's not about structure, but about the way we process requests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
8a4bf07
to
c3aa882
Compare
c3aa882
to
f370b86
Compare
rebased, |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: DirectXMan12, sttts 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 |
Marker syntax:
kubebuilder:pruning:PreserveUnknownFields
kubebuilder:validation:EmbeddedResource
Fixing #281