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

Test both tinygo and standard go #17

Merged
merged 3 commits into from
Oct 26, 2023
Merged

Test both tinygo and standard go #17

merged 3 commits into from
Oct 26, 2023

Conversation

egonelbre
Copy link
Contributor

This adds tests for both std go and tinygo.

It also does some project cleanup:

  • remove the .wasm files, because it looks like it's not necessary to commit them
  • remove .work file and example submodule, because they cause more problems than they solve (in this case)
  • move the examples to separate folders, because that allows different IDE-s and tooling to work with them nicely

@bhelx
Copy link
Contributor

bhelx commented Oct 16, 2023

You can pull my changes on .github/workflows/ci.yml to get the CI to go green.

Separating example into a different module makes life harder, without
significant benefits.
The new code should work with both Go and TinyGo.
Also move, then main.go to separate folders, that way different IDE-s
and tools work with them better.
@egonelbre
Copy link
Contributor Author

@bhelx note I've rebased the code.

@nilslice
Copy link
Member

Thank you!

I'm generally in favor of keeping .wasm in the repo (we always find a reason to download and use these modules) -- but not a requirement.

Would like to know what @bhelx / @zshipko prefer there before going one way or the other on committing the modules.

@egonelbre
Copy link
Contributor Author

@nilslice If they need to be downloaded one option would be to make them part of a release. (Rather than part of the source code)

@nilslice
Copy link
Member

Yea that's preferable -- especially since Go will pull down all of this into the system cache including the wasm.

@zshipko
Copy link
Contributor

zshipko commented Oct 24, 2023

Having the wasm modules in the repo is convenient, but can be pretty annoying to work with in git. Moving them to a release instead is a good idea!

@egonelbre
Copy link
Contributor Author

Do you want me to add the release thing in this PR or in a separate one?

@bhelx bhelx merged commit fc981a7 into extism:main Oct 26, 2023
2 checks passed
@bhelx
Copy link
Contributor

bhelx commented Oct 26, 2023

@egonelbre this looks good to me! Let's do follow ups in other PRs

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.

4 participants