-
-
Notifications
You must be signed in to change notification settings - Fork 231
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
Fix a dialyzer-issued warning #223
Fix a dialyzer-issued warning #223
Conversation
Are you using different Dialyzer flags perhaps? |
Just did some other changes and I'm not getting warnings on GitHub Actions without this PR. |
No different flags (using those from the project). But I was using OTP 24. Let me recheck this with a more recent OTP 24 version... The flag that controls this warning is Yup, still present (at least in OTP 24); let me recheck this with e.g. 23.2... no, not there. (this is where the PR must have been misleading, since I was under the impression I'd found this issue in a pre-23 version) Recap:
You decide if it's important to consider for now or leave it to handle later when we add 24 to CI. |
For reference, this is what I'm getting src/meck_proc.erl
Line 599 Column 5: Expression produces a value of type [{'error','non_existing' |
'not_main_node' | string() | {'encrypted_abstract_code',string()} |
{'no_abstract_code',string()} | {'no_file_attribute',string()} |
{'already_cover_compiled','no_beam_found',atom()}} | {'ok',atom()}] |
{'error','non_existing' | 'not_main_node' | string() | {'encrypted_abstract_code',string()} |
{'no_abstract_code',string()} | {'no_file_attribute',string()} |
{'already_cover_compiled','no_beam_found',atom()}} |
{'ok',atom()}, but this value is unmatched (there be newlines here, for readability)
|
@eproxus, don't close or merge this just yet. Let me confirm some stuff and report back here. |
(I'm thinking that either this was a hidden bug, now fixed, or the matching with |
|
I'm going to test my patience with |
Well, I couldn't find anything conclusive, but still get the warning on OTP 24. (compiling OTP locally, on a Mac, is sometimes painful 😢) |
Ok, let's leave this PR open and then if the warning shows up when OTP 24 is released we'll merge it? |
@eproxus, I'm OK with that (though it mostly postpones all our releases also, since we depend on |
After a short offline conversation with @eproxus he actually realised this might have surfaced a bug, so I adapted "my" solution to:
|
@eproxus, if you're OK with this would a new release be possible, to incorporate this change? Thanks much. |
As per @michalmuskala' help to find where the bug was fixed, here it is: erlang/otp@5040a7a.
|
@eproxus, since this is no longer an OTP-24 -related issue, do you think it possible to merge and release? Thanks. |
I'm not exactly sure how this got by GitHub Actions without triggering a fail. (I got it while preparing for OTP 24)