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
Remove old-style conversions registration #85891
Remove old-style conversions registration #85891
Conversation
da90fea
to
38bf08c
Compare
c.nameFunc = func(t reflect.Type) string { return "MyType" } | ||
|
||
// Ensure conversion funcs can call DefaultConvert to get default behavior, | ||
// then fixup remaining fields manually |
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.
This is (and already was) no longer true - the new-style conversion no longer can call DefaultConvert.
It seems DefaultConvert is not used anywhere, so I'm going to remove it in the subsequent PR.
@smarterclayton
@@ -47,19 +47,6 @@ func addToGroupVersion(scheme *runtime.Scheme) error { | |||
if err := scheme.AddIgnoredConversionType(&metav1.TypeMeta{}, &metav1.TypeMeta{}); err != nil { | |||
return err | |||
} | |||
err := scheme.AddConversionFuncs( |
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.
This isn't needed because below we're calling metav1.AddToGroupVersion
, which is registering all the necessary conversions (including these ones).
This PR may require API review. If so, when the changes are ready, complete the pre-review checklist and request an API review. Status of requested reviews is tracked in the API Review project. |
38bf08c
to
8010a7e
Compare
8010a7e
to
839b440
Compare
var scheme = runtime.NewScheme() | ||
|
||
// ParameterCodec knows about query parameters used with the meta v1beta1 API spec. | ||
var ParameterCodec = runtime.NewParameterCodec(scheme) |
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.
No one was using the ParameterCodec here?
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.
From v1beta1 - noone in k/k repo and anywhere else that I managed to check.
One question |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: smarterclayton, wojtek-t 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 |
/retest Review the full test history for this PR. Silence the bot with an |
2 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest |
/retest Review the full test history for this PR. Silence the bot with an |
2 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest |
/retest Review the full test history for this PR. Silence the bot with an |
8 similar comments
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
/retest Review the full test history for this PR. Silence the bot with an |
AddConversionFuncs in k8s.io/apimachinery/pkg/runtime.Scheme is deprecated and removed in k8s 1.18 (cf kubernetes/kubernetes#85891) This change replaces the deprecated registration code to this (more verbose) registration code, which will continue to be supported in K8s 1.18+. Signed-off-by: Olivier Lemasle <o.lemasle@gmail.com>
With this PR, we're removing the old-style conversion registration, e.g.
and the only allowed registration looks like:
Fix #84452