[GitHub] [trafficcontrol] rob05c commented on a change in pull request #5924: Add tool to migrate data between TV backends

[GitBox <gi...@apache.org>]

rob05c commented on a change in pull request #5924:
URL: https://github.com/apache/trafficcontrol/pull/5924#discussion_r652104208



##########
File path: lib/go-tc/deliveryservice_ssl_keys.go
##########
@@ -73,6 +73,7 @@ type SSLKeyRequestFields struct {
 	HostName     *string `json:"hostname,omitempty"`
 	Country      *string `json:"country,omitempty"`
 	State        *string `json:"state,omitempty"`
+	Version      *int    `json:"version,omitempty"`

Review comment:
       +1 - as long as it has a sane default, ideally as close to the prior behavior as possible.
   
   It technically can break code, e.g. `foo := Foo{a,b,c}` will break. But it's reasonable to add fields in minor versions, generally speaking, and any of the alternatives in Go aren't reasonable.
   
   So, as the person who's usually one of the biggest votes against breaking clients, IMO we should consider adding fields as non-breaking, and discourage clients from using that syntax (and the few others which break, which all have alternatives that don't).




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org