fix: OTP 21 compatibility #4
Conversation
| @@ -5,12 +5,14 @@ defmodule GcsSigner do | |||
|
|
|||
| @base_url "https://storage.googleapis.com" | |||
|
|
|||
| @otp_greater_21? :erlang.system_info(:otp_release) >= '21' | |||
There was a problem hiding this comment.
Is it possible to change single quote to double quote, since this project uses double quote as a convention?
There was a problem hiding this comment.
:erlang.system_info(:otp_release) returns an erlang charlist, which in elixir is denoted by single quotes. If I were to compare with a Binary (double quotes), the result would always be false:
iex> '21' >= "21"
false
There was a problem hiding this comment.
Ah I see! Thanks for the insight 😄
lib/gcs_signer.ex
Outdated
| content_type: String.t, | ||
| expires: integer | ||
| ] | ||
| verb: String.t(), |
There was a problem hiding this comment.
May I ask if there is a reason why indentation has changed here? I would prefer keeping the original indentation to match with other parts of our codebase.
lib/gcs_signer.ex
Outdated
| String.t, | ||
| sign_url_opts | ||
| ) :: String.t | ||
| %{client_email: String.t(), private_key: String.t()}, |
There was a problem hiding this comment.
Same with the change above - may I ask if there is a reason why indentation and method call format has changed here? I would prefer keeping the original indentation to match with other parts of our codebase.
lib/gcs_signer.ex
Outdated
| "Signature" => signature | ||
| } |> URI.encode_query | ||
|
|
||
| qs = |
There was a problem hiding this comment.
Separating righthand looks nice on the signature, so thank you for adjusting it.
On here however original code formatting feels better in terms of indentation. What do you think?
lib/gcs_signer.ex
Outdated
| @@ -66,18 +73,30 @@ defmodule GcsSigner do | |||
|
|
|||
| string | |||
| |> :public_key.sign(:sha256, private_key) | |||
| |> Base.encode64 | |||
| |> Base.encode64() | |||
There was a problem hiding this comment.
Can you remove the parenthesis here for consistency?
lib/gcs_signer.ex
Outdated
| private_key | ||
| else | ||
| private_key | ||
| # grab privateKey from the record tuple |
There was a problem hiding this comment.
I'd like to suggest moving this comment to above L96, since comment is in the middle of the pipe methods which makes private_key and subsequent method disconnected.
|
@hauptbenutzer I added lots of coding style change suggestions, if you feel it's quite overwhelming I'm also open to directly adding commits on your branch to make appropriate adjustments, and merge when it's done :) |
hauptbenutzer
force-pushed
the
feat/otp-21-compatiblity
branch
from
64853f9 to
851f496
Compare
|
@premist Sorry for that -- I had my editor configured to |
|
Thanks for your contribution! |
This fixes GCloud private key decoding for OTP 21, in which the decoding return format has changed.
See #3