[GitHub] [trafficcontrol] zrhoffman commented on a change in pull request #5248: Fix servers client methods changing call signatures

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

zrhoffman commented on a change in pull request #5248:
URL: https://github.com/apache/trafficcontrol/pull/5248#discussion_r518885076



##########
File path: lib/go-tc/servers.go
##########
@@ -574,11 +577,96 @@ func (s Server) ToNullable() ServerNullableV2 {
 	}
 }
 
+// ToNonNullable converts the ServerNullableV2 safely to a Server structure.
+func (s ServerNullableV2) ToNonNullable() Server {
+	coerceBool := func(b *bool) bool {
+		if b == nil {
+			return false
+		}
+		return *b
+	}
+	coerceInt := func(i *int) int {
+		if i == nil {
+			return 0
+		}
+		return *i
+	}
+	coerceString := func(s *string) string {
+		if s == nil {
+			return ""
+		}
+		return *s
+	}

Review comment:
       These functions should be declared at the package level for reuse. Go intentionally disallows declaring functions within other functions.

##########
File path: lib/go-tc/servers.go
##########
@@ -574,11 +577,96 @@ func (s Server) ToNullable() ServerNullableV2 {
 	}
 }
 
+// ToNonNullable converts the ServerNullableV2 safely to a Server structure.
+func (s ServerNullableV2) ToNonNullable() Server {

Review comment:
       In `atstccfg`, we have `toreq.ServersToNullable()`, which accepts and returns arrays of structs, not just a single one.
   
   https://github.com/apache/trafficcontrol/blob/1512b5ebf367466c60d70b239e680b2e04ee6ac2/traffic_ops_ort/atstccfg/toreq/toreq.go#L162-L164
   
   Should this too be a function that accepts and returns arrays of structs, rather than a method on `ServerNullableV2`?




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