ocket8888 commented on code in PR #7099: URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1025558560
########## traffic_ops/traffic_ops_golang/crconfig/deliveryservice.go: ########## @@ -145,7 +145,7 @@ FROM deliveryservice AS d INNER JOIN type AS t ON t.id = d.type LEFT OUTER JOIN profile AS p ON p.id = d.profile WHERE d.cdn_id = (select id FROM cdn WHERE name = $1) -AND d.active = true +AND d.active = 'ACTIVE' Review Comment: The point of using a constant is to avoid misspellings and so you can change it once to change it everywhere - but this being an enum type in the database as well as a Go constant sort of escapes both of those use cases. Because it's an enum it's not possible to insert a misspelling; that would break the API and fail tests etc. And also if the string used to represent "Active" ever changes, it's not possible to change it in only one place, since it would require a database migration. Plus, using string concatenation for building SQL queries - besides always making me a little nervous inherently - means that knowing what you're doing is safe depends on knowing the exact string value of the constant, so an abstraction could be detrimental to that. For all of those reasons I chose deliberately not to use the Go constant in queries. If you still want me to change it I will, I just wanted to explain my reasoning. -- 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]
