Skip to content

Feat: CLI polywrap codegen Command - #1051

Merged
dOrgJelli merged 18 commits into
origin-devfrom
feat/cli-codegen-command
Sep 7, 2022
Merged

Feat: CLI polywrap codegen Command#1051
dOrgJelli merged 18 commits into
origin-devfrom
feat/cli-codegen-command

Conversation

@dOrgJelli

@dOrgJelli dOrgJelli commented Jul 16, 2022

Copy link
Copy Markdown
Contributor

I think the CLI interface should be:

polywrap codegen
polywrap build
polywrap deploy
polywrap infra
polywrap create
polywrap run

Where each command decides internally what projects it works with.

This PR consolidates the polywrap plugin codegen and polywrap app codegen commands into a single polywrap codegen command. Additionally it refines best-practices about what options should be defined in polywrap manifest files and which are provided on the CLI.

NOTE: see the todo file for WIP impl notes

TODO:

  • option to override any property
  • option to specify a override fragment yaml file

closes: #1153

@dOrgJelli
dOrgJelli requested a review from namesty as a code owner July 16, 2022 11:28
@dOrgJelli dOrgJelli changed the title Feat: CLI polywrap codegen Command (plugin & app projects) Feat: CLI polywrap codegen Command Jul 16, 2022
@cbrzn
cbrzn marked this pull request as draft July 18, 2022 05:44
@pileks pileks linked an issue Aug 26, 2022 that may be closed by this pull request
@pileks pileks self-assigned this Sep 2, 2022
@pileks

pileks commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

polywrap codegen is now the only codegen command in the CLI right now. 😄

@dOrgJelli it seems that codegen outputs into ./src/wrap for all project types currently, so I have kept it at that.

Work that should be split into issues and done separately:

  • codegen for plugins appears to also output the plugins ABI into a ./build folder. We should refactor this out into the polywrap build command, and extend the build command to support plugins as well.
    • This would also move the --publish-dir option that the plugin codegen uses into build's --output-dir option
    • codegen would then support wasm, plugin and app project types, while build would support wasm, plugin and interface types
  • move --codegen-dir, --publish-dir, and --client-config into manifest files. Separate tasks for each of those might seem fair.
    • --codegen-dir issue created
    • --publish-dir issue created
    • --client-config issue created

@pileks

pileks commented Sep 2, 2022

Copy link
Copy Markdown
Contributor
  • move --codegen-dir, --publish-dir, and --client-config into manifest files. Separate tasks for each of those might seem fair.

    • --codegen-dir issue created
    • --publish-dir issue created
    • --client-config issue created

Note: we should probably rethink our manifest migration strategy before messing with manifests. While we can have all of these options set as optional manifest properties, I'd say that we'll likely want to create a proper migration strategy sooner than later, as it will reduce future work.

Current migration strategy:
Auto-generated code requires us to implement certain, fixed migrators.

available manifests:
0.1.0
0.2.0
0.3.0
0.4.0

migrators/migration paths:
0.1.0 -> 0.4.0
0.2.0 -> 0.4.0
0.3.0 -> 0.4.0

Proposed migration strategy:
Allow developers to declare their own migrators, from whichever version to whichever other version.
A simple search algorithm would then find the appropriate migration path, and could even allow for downgrades if the correct migrator exists.

available manifests:
0.1.0
0.2.0
0.3.0

migrators:
0.1.0 -> 0.2.0
0.2.0 -> 0.3.0
0.3.0 -> 0.4.0
0.1.0 -> 0.3.0

migration paths to 0.4.0:
0.1.0 -> 0.2.0 -> 0.3.0 -> 0.4.0
0.1.0 -> 0.3.0 -> 0.4.0

Related issue with further explanation:
#1211

@pileks

pileks commented Sep 2, 2022

Copy link
Copy Markdown
Contributor

Added issue for the splitting of plugin codegen behavior:
#1210

@pileks
pileks marked this pull request as ready for review September 2, 2022 18:35

@krisbitney krisbitney left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This PR is awesome. I think it's a very positive change. Thanks for working on this!

I'm concerned about the maintainability of getProjectFromManifest.ts. I refactored some it in another branch so you can see what I'm thinking. I messaged you.

Comment thread packages/cli/src/lib/project/helpers/getProjectFromManifest.ts Outdated
Comment thread packages/interfaces/file-system/package.json Outdated
@pileks
pileks requested a review from krisbitney September 5, 2022 17:29
krisbitney
krisbitney previously approved these changes Sep 6, 2022
@krisbitney

Copy link
Copy Markdown
Contributor

The dependencies in the other interfaces were also changed to 0.4.1

@dOrgJelli dOrgJelli left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Awesome work @pileks!

@dOrgJelli
dOrgJelli merged commit 165b176 into origin-dev Sep 7, 2022
@dOrgJelli
dOrgJelli deleted the feat/cli-codegen-command branch April 10, 2023 17:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Feat: CLI polywrap codegen command modification

4 participants