-
Notifications
You must be signed in to change notification settings - Fork 52
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
feat: add polywrap.yaml project resources #1430
Conversation
packages/test-cases/cases/cli/wasm/build-cmd/assemblyscript/015-resource-files/polywrap.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great in general, my only comment is more of a "big picture" issue.
packages/cli/src/lib/Compiler.ts
Outdated
// Output Polywrap Metadata | ||
await this._outputPolywrapMetadata(); | ||
// Copy: Resource files | ||
await this._copyResourceFiles(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it necessarily the Compiler's responsibility to copy resources and metadata?
IMO, strictly from a definition (what is a compiler?) standpoint, it would make more sense to move this into the build process itself (CLI, build strategy, something third).
Food for thought, and a possible refactor for later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep was thinking about this as well. Currently we have the metadata copy logic within the compiler, so that's why I added this there as well.
I'm planning on making a follow up PR that removes the polywrap.meta.yaml
manifest entirely, so that will be removed from the compiler.
Here's how I see it:
- Build process has multiple steps.
- The "primary" step is compiling the wrap.wasm module.
- Compiling the wrap.wasm module can be done using different "strategies" (local, vm)
- I would propose we organize our semantics like so:
build-steps/
codegen/
build-wrap-info/
build-wrap-wasm/
strategies/
vm/
local/
validate-wrap-wasm/
copy-resources/
These build steps can be functional units, that are configured by input parameters. This way you could execute an array of pre-configured build-steps in sequence, and they could become wrappers more easily (and user-configurable if desired).
Just a quick brainstorm on how we can flatten the architecture, make it more functional in nature, and allow for future extension.
No description provided.