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

Fixed an issue related to missing optional applications #514

Merged
merged 1 commit into from Aug 26, 2023

Conversation

GRoguelon
Copy link
Contributor

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 the deps_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

@axelson
Copy link
Contributor

axelson commented Aug 23, 2023

Thanks for this! I tested it on a work repository and it works as advertised 👍

This should be marked as Fixes #502 as well.

@jeremyjh jeremyjh force-pushed the fix/elixir-1-15-optional-apps branch from 2a671fe to ee52f60 Compare August 26, 2023 22:04
@jeremyjh jeremyjh merged commit 956c5d7 into jeremyjh:master Aug 26, 2023
12 checks passed
@jeremyjh
Copy link
Owner

Thanks!

@GRoguelon GRoguelon deleted the fix/elixir-1-15-optional-apps branch August 28, 2023 01:21
tombruijn added a commit to tombruijn/dialyxir that referenced this pull request Aug 28, 2023
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.
tombruijn added a commit to tombruijn/dialyxir that referenced this pull request Aug 28, 2023
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.
@whatyouhide
Copy link

@jeremyjh @GRoguelon this inadvertently bumps the Elixir requirement to 1.14+ because it uses then/2. @GRoguelon maybe you wanna send a PR that doesn't use then/2, so that this stays compatible with Elixir 1.6+, like mix.exs declares? 🙃

@jeremyjh
Copy link
Owner

@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?

@whatyouhide
Copy link

@jeremyjh what I’m saying is that if the library is only compatible with Elixir 1.15+, then we should change the requirement in mix.exs (here) to ~> 1.12. I see that you did that on the main branch, but the problem is that the 1.4.0 release declares it supports 1.6+, but it uses then/2, which was released in Elixir 1.12.

So right now it might be just a matter of releasing 1.4.1 as a bug fix 😉

@jeremyjh
Copy link
Owner

I released 1.4.1 last night.

@bryannaegele
Copy link

It would be nice if the change could be updated to not use then as while this project may only be testing 3 versions back, we have to support back further for some major libraries as we base off of supported OTP version rather than Elixir. We still have to support back to 1.11 for another year (OTP 27).

@David-Klemenc
Copy link
Contributor

Something like this:

== Compilation error in file lib/dialyxir/project.ex ==
Error: ** (CompileError) lib/dialyxir/project.ex:365: undefined function then/2
if System.version() |> Version.parse!() |> (&(&1.major >= 1 and &1.minor >= 15)).() do

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.

None yet

6 participants