-
Notifications
You must be signed in to change notification settings - Fork 66
Ffi to go adoption #3781
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
base: master
Are you sure you want to change the base?
Ffi to go adoption #3781
Conversation
herr-seppia
left a comment
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.
LGTM overall,
only thing is that you should rewrite the commit history to adapt to our standard:
- A commit cannot touch files belonging to different modules
- Every commit should be prefixed with the module name
- A CHANGELOG entry should be added
So, in your case should be something like:
workspace: add idea and c header to git ignorewallet-core: add support for GO FFI
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.
If this file is autogenerated, it should be ignored in git.
Furthermore, the command to generate it should be part of our Makefile
This can be done in a followup issue
| #[cfg(target_family = "wasm")] | ||
| let ptr = mem::malloc((4 + bytes.len()) as u32); | ||
| #[cfg(target_family = "wasm")] | ||
| let ptr = ptr as *mut u8; | ||
|
|
||
| #[cfg(not(target_family = "wasm"))] | ||
| let ptr = mem::allocate(4 + bytes.len()); | ||
|
|
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.
Let's open a followup issue to wrap this repeated code in a separate function
180d5ff to
884a623
Compare
884a623 to
76b491b
Compare
8314385 to
56866d3
Compare
No description provided.