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.

Sign up for GitHub

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

Jump to bottom

Fix WASM build and add a job in CI #195

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix WASM build and add a job in CI #195

wants to merge 2 commits into from

Conversation

RCasatta
Copy link
Collaborator

@RCasatta RCasatta commented Jan 23, 2024 •
edited
Loading

close #194

@RCasatta RCasatta marked this pull request as ready for review January 23, 2024 15:58
@RCasatta
Copy link
Collaborator Author

Note downstream you can use this without the dev-dep obviously, this is just needed to enforce wasm check in CI so that we are not breaking it in the future

apoelstra
apoelstra reviewed Aug 22, 2024
View reviewed changes
.github/workflows/rust.yml Outdated
- name: Checkout Toolchain
uses: dtolnay/rust-toolchain@stable
- run: rustup target add wasm32-unknown-unknown
- run: cargo check --target wasm32-unknown-unknown
Copy link
Member

Choose a reason for hiding this comment

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

In d6c5892:

trailing whitespace

Copy link
Member

Choose a reason for hiding this comment

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

@RCasatta ping -- if you fix the whitespace I can ack and merge this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

@apoelstra
Copy link
Member

LGTM, but would this check have actually caught the getrandom thing? My understanding is that if you compile without the js feature you'll get a runtime panic, so just running cargo check won't have noticed it.

Regardless, this CI job is an improvement over having nothing.

@RCasatta
Copy link
Collaborator Author

It seems it's a compile time check and without the getrandom thing the job would fail https://github.com/RCasatta/rust-elements/actions/runs/10578325188/job/29308193852?pr=2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@apoelstra apoelstra apoelstra left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Make it work on WASM
2 participants
@RCasatta @apoelstra