-
Notifications
You must be signed in to change notification settings - Fork 64
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Avoid deprecated http_uri functions for OTP 21+ #86
Conversation
In PR description you said
but in fact you are using it in OTP 21+. I think PR description is wrong, not the code (uri_string was introduced in OTP21). It looks good for me. Let's wait 1-2 days if anyone else have any comments. Otherwise I will merge it 04 Mar. |
Oops! I will fix the description ;) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a bunch @shino !
Left a few improvement comments, but I don't see any blockers.
%% Absolute file: | ||
{error, {no_default_port, file, Ref}} -> | ||
case parse_ref(RefBin) of | ||
{absolute, Ref} -> | ||
Ref; | ||
%% Relative |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove the %% Relative
comment. it is now nicely readable in the returned tuple 👏
@andreineculau Thanks a lot for your comments!! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should I merge it @andreineculau ?
Thank you @shino |
When compiling with OTP 23-rc1, deprecated warnings are emmited for two http_uri functions,
parse
anddecode
.In this PR:
parse
is replaced byuri_string:parse
.decode
is replaced by module privatehex_decode
function which is based onhttp_uri:decode
code, because I can not find alternative inuri_string
module.