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 back the API to take walrus module instead of wasm blob #23

Merged
merged 3 commits into from
Oct 6, 2022

Conversation

chenyan-dfinity
Copy link
Contributor

@chenyan-dfinity chenyan-dfinity commented Oct 6, 2022

Undo #19.

Decoupling walrus from ic-wasm API has the following drawbacks:

  • We need walrus::ModuleConfig to control how we parse the Wasm module. Taking a blob from the API cannot customize the parsing option
  • There are many use cases where we need to apply multiple transformations for the same Wasm module. With the blob API, we will have to re-parse the module several times
  • To avoid importing walrus from the SDK, all we need are the two util functions to parse wasm blob into Module, as added in this PR

@chenyan-dfinity chenyan-dfinity requested a review from a team as a code owner October 6, 2022 03:02
@lwshang lwshang merged commit 9d6e743 into main Oct 6, 2022
@lwshang lwshang deleted the undo-walrus branch October 6, 2022 18:25
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.

None yet

2 participants