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

Add elixir 1.18.0 compatibility #88

Merged
merged 6 commits into from
Dec 22, 2024
Merged

Conversation

icr4
Copy link
Contributor

@icr4 icr4 commented Dec 16, 2024

As mentioned in this issue required attribute in __info__/1 method has been deprecated.

This PR also fixes some warnings occurring in test environment regarding mismatching type issues.

@edgurgel
Copy link
Owner

Thanks @icr4!

We might need conditional tests depending on the elixir version 🤔 .

We can also add 1.18.0 to the list of versions that are tested on CI!

@icr4 icr4 changed the title Add elixir 1.18.0-rc.0 compatibility Add elixir 1.18.0 compatibility Dec 20, 2024
@icr4
Copy link
Contributor Author

icr4 commented Dec 20, 2024

Hi @edgurgel
I think i forgot to keep backward compatibility with Elixir 1.16 and 1.17
I've restored the previous code and ensured it compiles only with Elixir versions under 1.18

I preferred avoiding different version's code from compiling, but we might consider using Version.compare/2 instead to improve code readability.
What do you think?

Copy link
Owner

@edgurgel edgurgel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@edgurgel edgurgel merged commit 4b79392 into edgurgel:main Dec 22, 2024
4 checks passed
@georgeguimaraes
Copy link

tks @icr4!

@edgurgel can you launch a new version?

@edgurgel
Copy link
Owner

Yup a release should come out soon! I am just doing some tests with a few large internal projects. 🔜

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.

3 participants