api/types/container: InspectResponse: keep old name for embedded type #48124
Conversation
thaJeztah
added
area/api
status/2-code-review
area/go-sdk
|
LOL... I thought this was supported, but not for JSON in stdlib; golang/go#6213 (comment) So I guess we need to change the name back for now. |
thaJeztah
force-pushed
the
api_types_inspectbase
branch
from
c7a409a to
d976412
Compare
This is a follow-up to 1abc8f6, which moved the ContainerJSONBase to api/types/container, but also renamed it to container.InspectBase. This field is embedded into the InspectResponse type, which meant that renaming the type also implicitly renamed the field when creating this type from a struct-literal. While we're planning to merge these types (which would be a breaking change for users constructing it through struct-literals), let's keep it backward-compatible for now (other than deprecating the old names). We can continue the other changes separately. Signed-off-by: Sebastiaan van Stijn <[email protected]>
thaJeztah
force-pushed
the
api_types_inspectbase
branch
from
d976412 to
fb18966
Compare
| @@ -167,7 +167,7 @@ type InspectBase struct { | |||
| // InspectResponse is the response for the GET "/containers/{name:.*}/json" | |||
| // endpoint. | |||
| type InspectResponse struct { | |||
| *InspectBase | |||
| *ContainerJSONBase | |||
There was a problem hiding this comment.
Couldn't this also have a tag instead? *InspectBase `json:"ContainerJSONBase"?
(Is this really embedded into the JSON as "ContainerJSONBase": ...??? 😭)
There was a problem hiding this comment.
If not, I don't actually understand how that PR broke this nor how this PR fixes it. 🙈
|
Oh, you don't necessarily mean it was broken for JSON, just that JSON also doesn't support |
Yes, indeed; the previous change kept the JSON output the same, but it "broke" the Go side of things; users should still update their code to move to the new location, but with this PR at least it compiles (but printing "deprecation" warnings). Given that the intent is for this nested struct to Go away (which will be an intentionally breaking change), it's not worth breaking people twice just to have a nicer name for something that will go away. |
This is a follow-up to 1abc8f6 (#48057), which moved the ContainerJSONBase to api/types/container, but also renamed it to container.InspectBase. This field is embedded into the InspectResponse type, which meant that renaming the type also implicitly renamed the field when creating this type from a struct-literal.
While we're planning to merge these types (which would be a breaking change for users constructing it through struct-literals), let's keep it backward-compatible for now (other than deprecating the old names).
We can continue the other changes separately.
- A picture of a cute animal (not mandatory but encouraged)