Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Add WASM CI checks and make availability-store compile for WASM #626

Merged
merged 4 commits into from
Nov 29, 2019

Conversation

tomaka
Copy link
Contributor

@tomaka tomaka commented Nov 28, 2019

cc #625

The change to availability-store is kind of the Polkadot equivalent to paritytech/substrate#3957, except that we enforce this at compile time.

In order to prevent this from breaking accidentally, also adds CI checks.

@tomaka tomaka added the A0-please_review Pull request needs code review. label Nov 28, 2019
@tomaka tomaka requested a review from gabreal November 28, 2019 10:10
stage: test
<<: *test-refs
<<: *docker-env
<<: *compiler_info
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copy-pasted these lines but I have no idea what they are.

@tomaka
Copy link
Contributor Author

tomaka commented Nov 28, 2019

Oh. Interestingly, running cargo build --package foo fails whereas doing cd foo && cargo build works. Will investigate more.

@rphmeier
Copy link
Contributor

Why does the availability store need to compile to WASM?

@tomaka
Copy link
Contributor Author

tomaka commented Nov 28, 2019

Why does the availability store need to compile to WASM?

We need some conditional compilation somewhere, and where is a question of surprise factor/usability/cleanness.

Either we make availability-store compile for WASM, or we have to tweak the node in order to not compile or use availability-store.
Since it's going to be the same for other crates, the code of the node could become quite messy and difficult to read if all the conditional compilation happens at a single place.

@rphmeier
Copy link
Contributor

I don't follow, do you mean that this should be used in the runtime? Because especially after #345 that won't be possible

@tomaka
Copy link
Contributor Author

tomaka commented Nov 28, 2019

Ah no, this PR is about wasm-in-the-browser! Sorry for the confusion.

@gavofyork gavofyork merged commit 5598ed9 into paritytech:master Nov 29, 2019
@tomaka tomaka deleted the wasm-start branch November 29, 2019 16:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants