Skip to content

Conversation

@siexp
Copy link

@siexp siexp commented Jun 11, 2025

No description provided.

@siexp siexp requested review from Neotamandua and herr-seppia and removed request for Neotamandua June 11, 2025 10:43
Copy link
Member

@herr-seppia herr-seppia left a 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:

  1. A commit cannot touch files belonging to different modules
  2. Every commit should be prefixed with the module name
  3. A CHANGELOG entry should be added

So, in your case should be something like:

  • workspace: add idea and c header to git ignore
  • wallet-core: add support for GO FFI

Copy link
Member

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

Comment on lines 157 to 164
#[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());

Copy link
Member

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

@gootsolution gootsolution reopened this Jun 16, 2025
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