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

Move wasms to shared resources folder and clean up dockerfiles #229

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

spalladino
Copy link
Collaborator

@spalladino spalladino commented Apr 10, 2023

  • Moves wasm files from src (in both bb and circuits) to a resources folder, to be accessed from both src and dist. This removes the need for a manual post-build step to copy or symlink the wasm, which was required also when building a package that depended on these. Cleans up dockerfiles to remove these build steps. Fixes Relocate wasm artifact within ts packages #206.
  • Copies entire project to each dockerfile, so we don't need to keep dependencies in sync with each dockerfile as well. If performance becomes an issue here, we can change the prepare root script to also update dockerfiles. Fixes Do not use explicit dependencies on Dockerfile #224.

@spalladino spalladino marked this pull request as ready for review April 10, 2023 19:01
@spalladino spalladino requested review from spypsy and ludamad and removed request for spypsy April 10, 2023 19:01
Copy link
Member

@spypsy spypsy left a comment

Choose a reason for hiding this comment

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

think the resources idea is nice, no more having to worry about copying to dest, either in docker or during yarn build.
Not a big fan of copying everything in the Dockerfiles but if doesn't actually affect performance I don't see a reason to not do it

@spalladino
Copy link
Collaborator Author

Not a big fan of copying everything in the Dockerfiles but if doesn't actually affect performance I don't see a reason to not do it

We can always roll it back, as long as we automate the listing of deps to copy in the dockerfile!

@spalladino spalladino merged commit c7f17bf into master Apr 11, 2023
@spalladino spalladino deleted the palla/relocate-wasms branch April 11, 2023 11:55
@spalladino spalladino mentioned this pull request Apr 13, 2023
6 tasks
ludamad pushed a commit that referenced this pull request Apr 14, 2023
* chore(specs): remove

* chore(specs): remove

* revert bb change

---------

Co-authored-by: cheethas <[email protected]>
ludamad pushed a commit that referenced this pull request Apr 17, 2023
* chore(specs): remove

* chore(specs): remove

* revert bb change

---------

Co-authored-by: cheethas <[email protected]>
ludamad pushed a commit that referenced this pull request Apr 17, 2023
* chore(specs): remove

* chore(specs): remove

* revert bb change

---------

Co-authored-by: cheethas <[email protected]>
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.

Do not use explicit dependencies on Dockerfile Relocate wasm artifact within ts packages
2 participants