Fix incorrect useage of charlist #179
Conversation
Single quotes (charlists) were used instead of double quotes (binaries).
| optional(:tls_required) => boolean(), | ||
| optional(:auth_required) => boolean(), | ||
| } | ||
| :client_id => non_neg_integer(), |
There was a problem hiding this comment.
Looks like my formatter changed this as well. If you'd like, I can split this out into a different PR
There was a problem hiding this comment.
I thought I had a formatter check built into the CI process, but maybe I'm misremembering. If this is the standard formatter settings for current elixir versions then we should just apply it now.
There was a problem hiding this comment.
TBH i'm not sure where the difference is coming from
|
Hey @davydog187 thanks for opening a PR ❤️ 💛 💙 💚 💜 Most of these arguments do (or at least did) have a requirement to be passed a charlists because they are passthrough arguments to erlang stdlib functions (opening a TLS or TCP socket). Did you test passing in strings/binaries? Maybe the erlang functions have started to support strings? I'm definitely open to updating the docs and improving typespecs if the underlying functions support this. It's definitely more idiomatic for Elixir libraries |
|
Cleaning up now, sorry @mmmries I was moving too fast 😅 |
|
@mmmries regarding the question of binaries vs charlists, https://github.com/nats-io/nats.ex/blob/v1.9.0/lib/gnat/handshake.ex#L10 Technically it is correct to pass a a binary or a charlist, but the typespecs only specify |
|
Should be good to run CI now @mmmries |
|
Double thanks for taking a second pass and confirming the details on the binary vs charlists. Appreciate the contribution |
Single quotes (charlists) were used instead of double quotes (binaries).