Skip to content

Fixes for release - #237

Merged
Mec-iS merged 6 commits into
smartcorelib:developmentfrom
morenol:lmm/fixes
Nov 8, 2022
Merged

Mec-iS merged 6 commits into
smartcorelib:developmentfrom
morenol:lmm/fixes

Conversation

@morenol

@morenol morenol commented Nov 8, 2022

Copy link
Copy Markdown
Collaborator

Fixes #

Checklist

  • My branch is up-to-date with development branch.
  • Everything works and tested on latest stable Rust.
  • Coverage and Linting have been applied

Current behaviour

New expected behaviour

Change logs

@morenol
morenol marked this pull request as ready for review November 8, 2022 15:54
@morenol
morenol requested a review from Mec-iS as a code owner November 8, 2022 15:54
@Mec-iS

Mec-iS commented Nov 8, 2022 •

Copy link
Copy Markdown
Collaborator

I think you are overthinking it. It looks quite easy to me:

  • wasm32-unknown is for browsers
  • wasm32-wasi is for other VMs that have access to binary interfaces

Also, let's try to not duplicate pipelines in tests, once we run cargo tests --all-features for each target isn't it enough? What is CI / check_features (ubuntu, --features serde) (pull_request) about?

@morenol

morenol commented Nov 8, 2022 •

Copy link
Copy Markdown
Collaborator Author

I think you are overthinking it. It looks quite easy to me:

  • wasm32-unknown is for browsers
  • wasm32-wasi is for other VMs that have access to binary interfaces

Also, let's try to not duplicate pipelines in tests, once we run cargo tests --all-features for each target isn't it enough?

In Fluvio we use wasm32-unknown-unknown with wasmtime. I think that should not assume that thing since wasm32-unknown-unknown is supported by wasmtime

@Mec-iS Mec-iS mentioned this pull request Nov 8, 2022
@Mec-iS

Mec-iS commented Nov 8, 2022

Copy link
Copy Markdown
Collaborator

Interesting, which VMs you support for Wasm32? Why you don't use Wasi?

@Mec-iS
Mec-iS merged commit 9eaae9e into smartcorelib:development Nov 8, 2022
@morenol
morenol deleted the lmm/fixes branch November 8, 2022 16:07
@morenol

morenol commented Nov 8, 2022

Copy link
Copy Markdown
Collaborator Author

We recently added support for WASI, but still is not enabled by default, we could consider to migrate to only use WASI in the feature but that decission has not been taken yet

morenol added a commit that referenced this pull request Nov 8, 2022
* Fixes for release
* add new test
* Remove change applied in development branch
* Only add dependency for wasm32
* Update ci.yml

Co-authored-by: Luis Moreno <morenol@users.noreply.github.com>
Co-authored-by: Lorenzo <tunedconsulting@gmail.com>
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