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 workflow to automatically build WASM files #92

Merged
merged 15 commits into from
Feb 24, 2024
Merged

Add workflow to automatically build WASM files #92

merged 15 commits into from
Feb 24, 2024

Conversation

Mqxx
Copy link
Contributor

@Mqxx Mqxx commented Jan 24, 2024

Hey I recently opened this issue #91. I personally don't have much experience with WebAssembly or Rust. However, I have tried to create an automated workflow to create WASM bindings for this project on release.

This is the first time I contribute to such a large project.

@Mqxx Mqxx changed the title Added workflow to automatically build WASM files Add workflow to automatically build WASM files Jan 24, 2024
Katze719

This comment was marked as resolved.

Copy link

@Katze719 Katze719 left a comment

Choose a reason for hiding this comment

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

needs to be merged asap

@connorskees
Copy link
Owner

Awesome, thank you for opening this PR. This is something I've been thinking about setting up for a while. I'll take a look sometime this week.

@connorskees
Copy link
Owner

CI seems to be failing due to a malformed cargo.toml.

Ideally this action wouldn't run on every commit pushed to master, but rather on every release/tag created.

@Mqxx
Copy link
Contributor Author

Mqxx commented Jan 30, 2024

CI seems to be failing due to a malformed cargo.toml.

Ideally this action wouldn't run on every commit pushed to master, but rather on every release/tag created.

I am not sure what caused this. I have only appened my stuff to the Cargo.toml as you can see here:
5bb1623#diff-2e9d962a08321605940b5a657135052fbcef87b5e360662bb527c96d9a615542.

Copy link

@Katze719 Katze719 left a comment

Choose a reason for hiding this comment

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

@connorskees when do you want this github action to run? on every release? or tag push? and how are your tags looking? v*.*.*?

@connorskees
Copy link
Owner

@Katze719 Ideally every tag, as so far I haven't done any releases. You can see the tags in https://github.com/connorskees/grass/tags, I think I started with a v prefix but eventually dropped it.

@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 2, 2024

The reason why we ask this is because for the workflow to trigger you need to specify a pattern. Example: v*.*.*. I have updated the workflow so that it does trigger on every release/prerelease thats gets created.

.github/workflows/build_wasm.yml Outdated Show resolved Hide resolved
Co-authored-by: Katze719 <38188106+Katze719@users.noreply.github.com>
@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 2, 2024

@connorskees The workflow sould now trigger on every TAG pushed.

@Katze719
Copy link

Hey @connorskees I don't want to be intrusive, but can this pull-request be merged? Or is something still missing?

@connorskees
Copy link
Owner

Hi @Katze719, CI is failing. I think you'd need to change which Cargo.toml file you edit. You probably want the one inside lib/, but you could also make a 4th package if that doesn't work

@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 13, 2024

@connorskees Everything thould be fixed now. 👍

crates/lib/Cargo.toml Outdated Show resolved Hide resolved
@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 13, 2024

I have commented out the wasm-bindgen dependency because you are right, I dont need it.

@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 13, 2024

@connorskees I have updated everything. Should work now.

@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 19, 2024

@connorskees Is there still something missing? Or can this be merged?

@connorskees
Copy link
Owner

Looks good to me. Thanks again for your work on this!

@connorskees connorskees merged commit c887e17 into connorskees:master Feb 24, 2024
4 checks passed
@Mqxx
Copy link
Contributor Author

Mqxx commented Feb 24, 2024

Looks good to me. Thanks again for your work on this!

np 👍

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