Skip to content
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

Use == 0.0 to check if the value is zero instead of pattern matching to fix warnings in the OTP master #128

Merged
merged 1 commit into from
Sep 29, 2023

Conversation

sile
Copy link
Contributor

@sile sile commented Aug 14, 2023

Thank you for creating and maintaining this library.
I have discovered an issue whereby when compiling this library using the OTP master branch, the compiler emits the following warning.

$ rebar3 compile
===> Verifying dependencies...
===> Analyzing applications...
===> Compiling jesse
===> Compiling src/jesse_validator_draft4.erl failed
src/jesse_validator_draft4.erl:1019:5: matching on the float 0.0 will no longer also match -0.0 in OTP 27. If you specifically intend to match 0.0 alone, write +0.0 instead.

This PR fixes the warning.

@RoadRunnr
Copy link

@for-GET OTP 26.1 is out and jesse does not compile on it because of the float warning. What are the chances that this PR gets merged soonish?

@srijan
Copy link

srijan commented Sep 20, 2023

Until this (or some solution) is merged, this worked for me (in rebar.config)

{overrides, [
    {add, jesse, [{erl_opts, [nowarn_match_float_zero]}]}
]}.

@kivra-pauoli
Copy link

👋 @andreineculau, any chance this gets merged+tagged+released soonish, 🙏? Thanks.

@seriyps seriyps merged commit cf075d2 into for-GET:master Sep 29, 2023
@seriyps
Copy link
Contributor

seriyps commented Sep 29, 2023

I merged it. Thanks!

@sile sile deleted the fix-otp-master-warnings branch September 29, 2023 14:52
@kivra-pauoli
Copy link

@seriyps, thanks a lot. Any chance it gets tagged + Hex publish?

@seriyps
Copy link
Contributor

seriyps commented Oct 2, 2023

I see that we have some CI issues, so, it would be nice to fix that before publishing

@kivra-pauoli
Copy link

@seriyps, any idea why CI didn't run in this pull request?

@seriyps
Copy link
Contributor

seriyps commented Nov 21, 2023

@kivra-pauoli sorry, I think GitHub automatically disabled it because periodic "crontab" job was failing for a long period of time

@zmstone
Copy link

zmstone commented Nov 28, 2023

when can I expect a new release with this fix included ?

This was referenced Feb 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants