take out osb prefixes through rote transformation of variable names and annotations #843
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
|
anybody know where |
|
@MHBauer I believe |
|
wanna remove MaxDBsPerNode as part of this? |
I'd really prefer to just make this one transformations. If we start accreting other changes into this, it'll be much harder to review. |
|
I think this is done for now. |
| // Description is a short description of the service. | ||
| Description string `json:"description"` | ||
|
|
||
| // Metadata fields | ||
| OSBMetadata *runtime.RawExtension `json:"osbMetadata, omitempty"` | ||
| Metadata *runtime.RawExtension `json:"externalMetadata, omitempty"` |
There was a problem hiding this comment.
ServiceClass already has a metadata field from the kubernetes ObjectMeta. I pseudocopied the ExternalID idea and added an External prefix. This was after I had renamed the field itself to Metadata.
There was a problem hiding this comment.
@MHBauer unfortunately, I think we should probably call the field ExternalMetadata because ServiceClass embeds the ObjectMeta type, which as you know has the Metadata field. So, when programming to the API, the Metadata field is going to be ambiguous versus the ObjectMeta field with the same name.
So, I think we should call the ServiceClass and ServicePlan fields ExternalMetadata
There was a problem hiding this comment.
+1 plus I'm not keen on the json name and golang name being so different.
| // Description is a short description of the service. | ||
| Description string `json:"description"` | ||
|
|
||
| // Metadata fields | ||
| OSBMetadata *runtime.RawExtension `json:"osbMetadata, omitempty"` | ||
| Metadata *runtime.RawExtension `json:"externalMetadata, omitempty"` |
There was a problem hiding this comment.
@MHBauer unfortunately, I think we should probably call the field ExternalMetadata because ServiceClass embeds the ObjectMeta type, which as you know has the Metadata field. So, when programming to the API, the Metadata field is going to be ambiguous versus the ObjectMeta field with the same name.
So, I think we should call the ServiceClass and ServicePlan fields ExternalMetadata
pkg/apis/servicecatalog/types.go
Outdated
| @@ -153,10 +153,10 @@ type ServicePlan struct { | |||
| ExternalID string | |||
|
|
|||
| // OSB-specific | |||
| OSBFree bool | |||
| Free bool | |||
There was a problem hiding this comment.
What do people think about calling this IsFree ?
|
@vaikas-google nope, that one just removes a couple of fields, looks like morgan will have to rebase onto it |
|
@MHBauer LGTM modulo |
check v1alpha1/types.go for the names and annotation changes
|
Rebased. Should build and stuff. Codegen struggles. |
|
LGTM if CI is happy but I still prefer |
We have an unwritten convention not to use 'Is' as a prefix for boolean fields, which I opened a PR to document: kubernetes/community#624 |
check v1alpha1/types.go for the names and annotation changes
want to make sure the this gets tested in ci stuff
pkg/apis/servicecatalog/v1alpha1/types.go is what you should look at. everythign else is based on that or generated code. specifically confirm the annotations changes.
Closes #834