Deduced types #58
Deduced types #58
Conversation
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: apelisse 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 |
k8s-ci-robot
added
cncf-cla: yes
|
/assign @jennybuckley |
|
I'm writing tests now. |
k8s-ci-robot
added
size/L
Some types don't have a schema (CRDs), and so we're creating the schema on the fly for these values. We also build the infrastructure to create these types easily and similarly to how we handled existing types.
Since we may not need the schema at all to resolve the path (and now we may not even have it), let's not dereference it until necessary.
apelisse
force-pushed
the
deduced-types
branch
from
93532cd to
b1fda53
Compare
k8s-ci-robot
added
size/XL
|
|
||
| // DeducedParseableType is a ParseableType that deduces the type from | ||
| // the content of the object. | ||
| type DeducedParseableType struct{} |
There was a problem hiding this comment.
Why is DeducedParseableType exported, but parsableType isn't?
There was a problem hiding this comment.
Is it because this type is just a struct{} alias?
There was a problem hiding this comment.
Because the other one needs to be created in a "fancy way", so it's build through a method that returns the interface?
I could do
func NewDeducedParseableType() ParseableType {
return deducedParseableType{}
}
but I don't think it's very useful
There was a problem hiding this comment.
in fewer words, yes =)
| FromUnstructured(interface{}) (TypedValue, error) | ||
| } | ||
|
|
||
| type parseableType struct { |
There was a problem hiding this comment.
Maybe this could be named, schemaParseableType, explicitParseableType or something else (neither of those sound too good to me), to indicate that this one comes from a schema?
There was a problem hiding this comment.
Yeah, it's private though so it's less critical?
|
/lgtm |
k8s-ci-robot
added
the
lgtm
No description provided.