-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
🏃 Use scheme.Convert to convert unstructrured objects #779
🏃 Use scheme.Convert to convert unstructrured objects #779
Conversation
/assign @gerred @DirectXMan12 |
9c25343
to
a2070f7
Compare
a2070f7
to
c8a7522
Compare
/retest |
@vincepri my comments are generally around documentation because I agree with the organization here. Given I'm in queue to be an approver, I will save my opinion for approval and would ask @alenkacz, @shawn-hurley, @alvaroaleman, @kensipe, or @joelanford to provide the proper lgtm. |
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.
c8a7522
to
0df2e7b
Compare
Fixed both comments, 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.
/lgtm
thanks :)
Update kubebuilder book for typos and repaired links
@gerred ptal |
Pending @DirectXMan12 and @vincepri updating on our API versions, I'm: /approve Since we have requested changes, this is not a final approve, but rather a NB 👍 in lieu of other perspectives convincing me on this change. :) |
Signed-off-by: Vince Prignano <vincepri@vmware.com>
0df2e7b
to
5c49cc4
Compare
@gerred @DirectXMan12 imports have been fixed to be explicit, ptal :) |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: gerred, vincepri 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 |
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
Signed-off-by: Vince Prignano vincepri@vmware.com
This PR updates the envtest helper to use a local scheme and the
Convert
method, which should preserve the TypeMeta information.runtime.DefaultUnstructuredConverter
doesn't know how to populateTypeMeta
information from the Go Struct, whilescheme.Convert
uses internal helper methods to figure that out.From a UX point of view, this change has the benefit that
envtest
users can use the Go structs without worrying about TypeMeta. If TypeMeta isn't present when we install CRDs, the installation will fail andenvtest
won't start.