Copilot commented on code in PR #161:
URL: https://github.com/apache/cloudstack-go/pull/161#discussion_r3719309794
##########
generate/generate.go:
##########
@@ -2009,6 +2009,20 @@ func (s *service) generateResponseType(a *API) {
pn("")
return
}
+ if a.Name == "listVnfAppliances" {
+ // The API docs do not describe the shape of the "vnfnics"
field, so this
+ // type is hand maintained to mirror
org.apache.cloudstack.api.response.VnfNicResponse.
+ pn("type VnfNic struct {")
+ pn(" Deviceid int64 `json:\"deviceid\"`")
+ pn(" Description string `json:\"description\"`")
+ pn(" Management bool `json:\"management\"`")
+ pn(" Name string `json:\"name\"`")
+ pn(" Networkid string `json:\"networkid\"`")
+ pn(" Networkname string `json:\"networkname\"`")
+ pn(" Required bool `json:\"required\"`")
+ pn("}")
+ pn("")
+ }
Review Comment:
The generator only emits the `VnfNic` type when `a.Name ==
\"listVnfAppliances\"`, but `mapType` now maps any `vnfnics` field to
`[]*VnfNic`. If an API spec/version includes `vnfnics` but does not include
`listVnfAppliances`, the generated code will reference `VnfNic` without
defining it, causing a compile error. Consider emitting `VnfNic`
unconditionally in a shared/common types section, or tracking whether `vnfnics`
was encountered and emitting the type once whenever it’s needed (independent of
a specific API name).
##########
generate/generate.go:
##########
@@ -2403,6 +2417,9 @@ func mapType(aName string, pName string, pType string)
string {
if pName == "scaledownpolicies" || pName == "scaleuppolicies" {
return "[]*AutoScalePolicy"
}
+ if pName == "vnfnics" {
+ return "[]*VnfNic"
+ }
Review Comment:
The generator only emits the `VnfNic` type when `a.Name ==
\"listVnfAppliances\"`, but `mapType` now maps any `vnfnics` field to
`[]*VnfNic`. If an API spec/version includes `vnfnics` but does not include
`listVnfAppliances`, the generated code will reference `VnfNic` without
defining it, causing a compile error. Consider emitting `VnfNic`
unconditionally in a shared/common types section, or tracking whether `vnfnics`
was encountered and emitting the type once whenever it’s needed (independent of
a specific API name).
##########
cloudstack/VirtualNetworkFunctionsService.go:
##########
@@ -3009,6 +3009,16 @@ func (s *VirtualNetworkFunctionsService)
ListVnfAppliances(p *ListVnfAppliancesP
return &r, nil
}
+type VnfNic struct {
+ Deviceid int64 `json:"deviceid"`
+ Description string `json:"description"`
+ Management bool `json:"management"`
+ Name string `json:"name"`
+ Networkid string `json:"networkid"`
+ Networkname string `json:"networkname"`
+ Required bool `json:"required"`
+}
Review Comment:
`VnfNic` is used broadly (multiple services now reference it), but it’s
defined inside `VirtualNetworkFunctionsService.go`, which makes the type harder
to discover and creates implicit coupling between otherwise unrelated services.
Consider placing `VnfNic` in a shared/package-level types file (e.g.,
`cloudstack/types.go`) that’s clearly intended for cross-service DTOs, and
updating the generator to emit it there.
##########
cloudstack/VirtualMachineService.go:
##########
@@ -4800,7 +4800,7 @@ type VirtualMachine struct {
Videoram int64 `json:"videoram"`
Vmtype string `json:"vmtype"`
Vnfdetails map[string]string `json:"vnfdetails"`
- Vnfnics []string `json:"vnfnics"`
+ Vnfnics []*VnfNic `json:"vnfnics"`
Review Comment:
Changing `vnfnics` from `[]string` to `[]*VnfNic` across many exported
response/resource structs is a breaking public API change for downstream
consumers. If backward compatibility is required, consider introducing a
dedicated `Vnfnics` type with custom JSON unmarshalling that can accept both
legacy (`[]string`) and structured (array of objects) shapes, or providing an
alternate accessor while keeping the existing field shape.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
