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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
