-
-
Notifications
You must be signed in to change notification settings - Fork 82
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
Support Elixir 1.15 #261
Support Elixir 1.15 #261
Conversation
e7de73b
to
2c22497
Compare
9067a63
to
739c49a
Compare
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
8a3f28a
to
1d0b541
Compare
1d0b541
to
bb4217c
Compare
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.
Looking pretty good.
I don't like the if statements in the tests, but I don't know of a better way to do it.
Just a couple comments.
apps/remote_control/test/lexical/remote_control/build/error_test.exs
Outdated
Show resolved
Hide resolved
Starting with Elixir 1.15, we don't need to capture io for compiling individual files or quoted anymore, because with_diagnostics returns all compile-time diagnostics. So I have implemented two ways to compile and process the compiled results. You can see from the tests that the location of the undefined diagnostic is more precise, and I've also enhanced the unused and undefined messages to make them more concise and clear.
to make it easier to test multiple versions, we do not need this file.
This commit only for version debuging, can delete when ready for merging.
I almost didn't change any *product* code of compiling project, they changed the Mix.Tasks, so we have to change these tests.
5fca3d1
to
1b68fd2
Compare
Thought: There is a lot of version checking sprinkled throughout the codebase now, I don't like that. I think instead, we should be checking for features, or at least indicate that we care about some feature being present. To that end, I suggest we write a defmodule Elixir.Features do
def with_diagnostics? do
Version.match?(System.version(), ">= 1.15.3")
end
end Then delegate to that. This also makes testing easier because you can patch that module to turn off a specific feature. |
|
we should be checking for features
30e3e82
to
e0960d7
Compare
e0960d7
to
5c6bb2b
Compare
Version.match?(System.version(), ">= 1.15.3") | ||
end | ||
|
||
def compile_wont_change_directory? do |
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.
flagging for a follow up commit: compile_keeps_current_directory?
Fixes #101
Closes #243
Starting with Elixir 1.15, we don't need to capture io for compiling individual files or quoted anymore,
because with_diagnostics returns all compile-time diagnostics.
So I have implemented two ways to compile and process the compiled results.
You can see from the tests that the location of the undefined diagnostic is more precise,
and I've also enhanced the unused and undefined messages to make them more concise and clear.
Showcase
Undefined
Unused
Known issues
Fixed by 1.15.4NAMESPACE
works very well, butmix release lexical
won't work onlexical
project.compile twice. no idea about this issue, becauseFixed by 1.15.4mix deps.compile
in command line has this problem too.We should wait until 1.15.3, because there are some bugs in1.15.2
, Elixir core team already fixed them: Clarify that Code.with_diagnostics/2 does not rescue elixir-lang/elixir#12742 (comment).