Use net/http constants instead of string literals for HTTP verbs
#587
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Switches from string literals like `"GET"` over to their corresponding constants in `net/http` like `http.MethodGet`. This isn't a huge win, but seems like slightly better hygiene and slightly reduces the risk of typos because a mistyped verb will now not compile instead of crashing on a test or at runtime.
| @@ -1,6 +1,8 @@ | |||
| package account | |||
|
|
|||
| import ( | |||
| "net/http" | |||
There was a problem hiding this comment.
Just to possibly pre-empt some feedback: net/http is new in all client packages, but they were already depending on it transitively through the top-level stripe package, so I don't think including it in all of these will have any meaningful effect on final binary size.
| @@ -240,7 +240,7 @@ func (s *BackendConfiguration) CallRaw(method, path, key string, form *form.Valu | |||
| var body io.Reader | |||
| if form != nil && !form.Empty() { | |||
| data := form.Encode() | |||
| if strings.ToUpper(method) == "GET" { | |||
| if method == http.MethodGet { | |||
There was a problem hiding this comment.
I stripped the ToUpper here — all calls now use net/http constants so it should be perfectly safe to just compare it directly against http.MethodGet now.
|
Thank you as usual Remi! |
|
Released as 34.3.0. |
nadaismail-stripe
pushed a commit
that referenced
this pull request
Oct 18, 2024
* added fixes for security scan issues * fixed empty exception list check * changed empty list check order * fixing deprecated var name * fix for failing test class
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Switches from string literals like
"GET"over to their correspondingconstants in
net/httplikehttp.MethodGet. This isn't a huge win,but seems like slightly better hygiene and slightly reduces the risk of
typos because a mistyped verb will now not compile instead of crashing
on a test or at runtime.
For review: I basically did a project-wide find and replace, so me
accidentally screwing up a verb change is relatively unlikely.
r? @remi-stripe
cc @stripe/api-libraries