-
-
Notifications
You must be signed in to change notification settings - Fork 142
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
Fixed an issue related to missing optional applications #514
Fixed an issue related to missing optional applications #514
Conversation
Thanks for this! I tested it on a work repository and it works as advertised 👍 This should be marked as Fixes #502 as well. |
2a671fe
to
ee52f60
Compare
Thanks! |
Since PR jeremyjh#514, the `then` macro is used, which was introduced in Elixir 1.12.0. If installed on older Elixir versions, it will fail to compile with the error below. ``` ==> dialyxir Compiling 66 files (.ex) == Compilation error in file lib/dialyxir/project.ex == ** (CompileError) lib/dialyxir/project.ex:365: undefined function then/2 (elixir) expanding macro: Kernel.|>/2 lib/dialyxir/project.ex:365: Dialyxir.Project (module) (elixir) expanding macro: Kernel.if/2 lib/dialyxir/project.ex:365: Dialyxir.Project (module) ``` Bump the minimal required Elixir version to communicate older versions are no longer supported. Elixir 1.12 is also the oldest version tested in the CI.
Since PR jeremyjh#514, the [`Kernel.then/2` macro](https://hexdocs.pm/elixir/Kernel.html#then/2) is used, which was introduced in Elixir 1.12.0. If installed on older Elixir versions, it will fail to compile with the error below. ``` ==> dialyxir Compiling 66 files (.ex) == Compilation error in file lib/dialyxir/project.ex == ** (CompileError) lib/dialyxir/project.ex:365: undefined function then/2 (elixir) expanding macro: Kernel.|>/2 lib/dialyxir/project.ex:365: Dialyxir.Project (module) (elixir) expanding macro: Kernel.if/2 lib/dialyxir/project.ex:365: Dialyxir.Project (module) ``` Bump the minimal required Elixir version to communicate older versions are no longer supported. Elixir 1.12 is also the oldest version tested in the CI.
@jeremyjh @GRoguelon this inadvertently bumps the Elixir requirement to 1.14+ because it uses |
@whatyouhide I'm not sure this release is needed for older versions of Elixir, its mostly compatibility fixes for 1.15. I only test on the last 3 major versions. But, if its just using this one language feature thats an issue, maybe I should re-think that. Do you have projects stuck on older versions of Elixir? |
@jeremyjh what I’m saying is that if the library is only compatible with Elixir 1.15+, then we should change the requirement in So right now it might be just a matter of releasing 1.4.1 as a bug fix 😉 |
I released 1.4.1 last night. |
It would be nice if the change could be updated to not use |
Something like this:
if System.version() |> Version.parse!() |> (&(&1.major >= 1 and &1.minor >= 15)).() do
|
The PR updates the deps_app/2 function in the Dialyxir.Project module in
lib/dialyxir/project.ex
to handle different versions of Elixir. Added version checking for Elixir 1.15 or later, and adjusted the implementation accordingly. The changes ensure that thedeps_app/2
function works correctly with the appropriate version of Elixir.I'm not totally sure if my change is backward compatible with previous versions of Elixir. Also not sure if there is a better option to avoid the version check.
I used this patch for an Elixir/Phoenix application on 1.15 for several weeks without any issues and no warnings about optional applications.
Closes #509