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

Rename NativeFunc to TypedFunction #2933

Conversation

silwol
Copy link
Contributor

@silwol silwol commented Jun 6, 2022

Description

Review

  • Add a short description of the change to the CHANGELOG.md file

@silwol silwol force-pushed the dev/2915-rename-nativefunc-to-typedfunction branch from e30098b to ec317ba Compare June 6, 2022 13:32
@silwol silwol force-pushed the dev/2915-rename-nativefunc-to-typedfunction branch from ec317ba to 18d17d5 Compare June 6, 2022 13:35
Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

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

We need to still make NativeFunc type alias work (in case someone wants to import, we should just show a deprecation warning)

@silwol
Copy link
Contributor Author

silwol commented Jun 6, 2022

@syrusakbary It should work, see lib/api/src/js/mod.rs:87 and lib/api/src/sys/mod.rs:132.

I initially put the aliases directly next to the structs, but that caused a compiler warning when re-exporting them inside {sys,js}/mod.rs, so I moved the alias there instead of the re-export. The public API should work like before if I'm not completely wrong, because the sys::native and js::native mods are not directly visible to users of the API.

@syrusakbary syrusakbary marked this pull request as ready for review June 6, 2022 16:58
@syrusakbary syrusakbary merged commit 0516ddb into wasmerio:wasmer3 Jun 6, 2022
@silwol silwol deleted the dev/2915-rename-nativefunc-to-typedfunction branch June 7, 2022 04:42
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