Conversation
src/http/client.cr
Outdated
| # ``` | ||
| def post_form(path, form : Hash, headers : HTTP::Headers? = nil) : HTTP::Client::Response | ||
| body = HTTP::Params.from_hash(form) | ||
| def post_form(path, form : Hash|NamedTuple, headers : HTTP::Headers? = nil) : HTTP::Client::Response |
There was a problem hiding this comment.
Format check failed, I believe spaces are required on type unions (Hash | NamedTuple)
There was a problem hiding this comment.
Thanks, got a bit rusty I guess :)
9c81a02 to
e6ba387
Compare
|
|
|
@RX14 I find
I think in this case, |
|
@luislavena The consistency between them is that they have the type it's coming from in the same. They sound fine, from on it's own feels weird to me. Why not 2 seperate methods: |
|
What about |
|
Both How about we overload |
|
Oooh... I thought it returned an |
|
Or just |
e6ba387 to
07f0e85
Compare
src/http/params.cr
Outdated
| def self.from_hash(hash : Hash) | ||
| # Returns the given key value pairs as a | ||
| # url-encoded HTTP form/query. | ||
| def self.encode(hash : Hash) |
There was a problem hiding this comment.
Maybe we should restrict to Hash(String, _). I know it isn't part of the initial PR intent.
If we want to work with any type of hash we can merge the implementation.
If we want to support only String key hashes we could restrict here and catch the compile time error sooner.
Hash(String, _) sounds fine to me.
for NamedTuple Also allow passing NamedTuple to the corresponding post_form overloads on HTTP::Client
07f0e85 to
3bacc30
Compare
|
Alright, added the more specific restriction. |
|
Is this PR GTG? |
…s renamed to encode in crystal-lang#4205
…s renamed to encode in #4205
Also allow passing NamedTuple to the corresponding post_form overloads
on HTTP::Client
That should make passing some static form data (or even some data with static keys) more pleasant without sacrificing on performance as much.