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

Term Type #538

Merged
merged 1 commit into from
Aug 24, 2023
Merged

Term Type #538

merged 1 commit into from
Aug 24, 2023

Conversation

filmor
Copy link
Member

@filmor filmor commented May 14, 2023

Raise default version to 2.15 and implement get_type using enif_term_type

#509

@filmor filmor requested a review from a team May 25, 2023 08:50
@filmor filmor marked this pull request as ready for review June 13, 2023 06:01
@filmor
Copy link
Member Author

filmor commented Jun 13, 2023

The main disadvantage of this approach is that this actually breaks the API since enif_term_type distinguishes float and integer, but doesn't distinguish list and empty_list anymore.

Copy link
Member

@evnu evnu left a comment

Choose a reason for hiding this comment

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

I think this is fine! I only wonder if we should rid ourselves of NIF versions below 2.15, as that falls outside the OTP versions we support I think.

@evnu
Copy link
Member

evnu commented Aug 23, 2023

Seems that I forgot to save this comment:

Do we actually need to support nif versions below 2.15? Our README states that we support the three most-recent OTP releases, so I think that we can expect users to have at least NIF version 2.15. Note that we have an inconsistency in the README however:

The minimal supported NIF version for a library should be defined via Cargo features. The default is currently 2.14 (Erlang/OTP 21). To use features from NIF version 2.16 (Erlang/OTP 24), the respective feature flag has to be enabled on the dependency:

@filmor
Copy link
Member Author

filmor commented Aug 23, 2023

Good point, I'll adjust the README for this branch.

Let's release this (after the other pending PRs are processed) and drop 2.14 support after that.

@filmor filmor merged commit 05a5080 into rusterlium:master Aug 24, 2023
@filmor filmor deleted the term-type branch August 24, 2023 08:13
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.

2 participants