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

feat: add polywrap.yaml project resources #1430

Merged
merged 6 commits into from
Nov 21, 2022

Conversation

dOrgJelli
Copy link
Contributor

No description provided.

pileks
pileks previously approved these changes Nov 21, 2022
Copy link
Contributor

@pileks pileks left a 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.

// Output Polywrap Metadata
await this._outputPolywrapMetadata();
// Copy: Resource files
await this._copyResourceFiles();
Copy link
Contributor

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.

Copy link
Contributor Author

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.

@dOrgJelli dOrgJelli merged commit 698d547 into origin-dev Nov 21, 2022
@dOrgJelli dOrgJelli deleted the polywrap-project-resources branch April 10, 2023 17:05
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