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

Migrated code generation logic to it's own crate #890

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

ianmarmour
Copy link
Contributor

@ianmarmour ianmarmour commented Feb 16, 2025

Description of the change

Splits Code Generation logic out into it's own crate (from CLI).

Things I attempted to do pro-actively here,

  • Remove unnecessary dependencies from both the javy-codegen crate and the cli crate.
  • Added documentation and ensured all public interfaces are documented in javy-codegen.
  • Re-exported commonly used structs for ease of use.

Things we might want to change/add,

  • Crate specific tests ? (Maybe)
  • ???

Why am I making this change?

To enable runtime generation of WASM modules without shelling out to a CLI.

Checklist

  • I've updated the relevant CHANGELOG files if necessary. Changes to javy-cli and javy-plugin do not require updating CHANGELOG files.
  • I've updated the relevant crate versions if necessary. Versioning policy for library crates
  • I've updated documentation including crate documentation if necessary.

Related PR's

877, 879

@ianmarmour ianmarmour marked this pull request as ready for review February 16, 2025 00:32
@jeffcharles jeffcharles mentioned this pull request Feb 18, 2025
3 tasks
Copy link
Collaborator

@jeffcharles jeffcharles left a comment

Choose a reason for hiding this comment

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

Can we add the crate feature we talked about before so we can reduce visibility for the methods for working with the v2 and default plugins to the CLI crate?

Otherwise, I think it's a structurally sound change besides removing some methods from the public API.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a step to run tests for the codegen crate similar to the Test CLI step and a step to lint the codegen crate similar to the CLI crate?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we add a target for testing the codegen crate and add it as a prerequisite for the tests target?

crates/codegen/Cargo.toml Show resolved Hide resolved
crates/codegen/README.md Show resolved Hide resolved
crates/codegen/Cargo.toml Show resolved Hide resolved
crates/cli/benches/benchmark.rs Show resolved Hide resolved
crates/codegen/src/js.rs Show resolved Hide resolved
crates/codegen/src/plugin.rs Show resolved Hide resolved
crates/codegen/src/wit.rs Show resolved Hide resolved
crates/codegen/src/lib.rs Show resolved Hide resolved
@jeffcharles jeffcharles mentioned this pull request Feb 18, 2025
3 tasks
@ianmarmour
Copy link
Contributor Author

Can we add the crate feature we talked about before so we can reduce visibility for the methods for working with the v2 and default plugins to the CLI crate?

Otherwise, I think it's a structurally sound change besides removing some methods from the public API.

Totally forgot, of course!

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.

2 participants