[GitHub] [trafficcontrol] ocket8888 commented on a diff in pull request #7099: Delivery Service Active Flag Rework

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

ocket8888 commented on code in PR #7099:
URL: https://github.com/apache/trafficcontrol/pull/7099#discussion_r1005057324


##########
traffic_ops/traffic_ops_golang/deliveryservice/deliveryservices.go:
##########
@@ -138,7 +162,7 @@ func CreateV30(w http.ResponseWriter, r *http.Request) {
 
 	ds := tc.DeliveryServiceV30{}
 	if err := json.NewDecoder(r.Body).Decode(&ds); err != nil {
-		api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, errors.New("decoding: "+err.Error()), nil)
+		api.HandleErr(w, r, inf.Tx.Tx, http.StatusBadRequest, fmt.Errorf("decoding: %w", err), nil)

Review Comment:
   > Why the change?
   
   Because it preserves the identity of the underlying error that way. Here's [a playground link to an example program that shows what that means](https://go.dev/play/p/GOkCOgci1lh). It accomplishes the same thing as `errors.New(someString+err.Error())` but without destroying any information.
   
   > ...shouldn't we refactor others in the code base too?
   
   In my opinion, yes, and I always request people do it this way in my reviews. I only changed it in the files I was already editing, though, because this is done untold thousands of times throughout ATC. Nobody would ever review that PR. So I just make smaller changes to files as I work on them.



-- 
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: issues-unsubscribe@trafficcontrol.apache.org

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