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

Make event loop a runtime config instead of a Cargo feature #838

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

jeffcharles
Copy link
Collaborator

Description of the change

Changes experimental_event_loop from a Cargo feature to a runtime config. This also introduces a javy_plugin_api::Config that wraps the javy::Config so we can add configuration options that only affect the javy_plugin_api crate behaviour and that don't inherently change how the javy::Runtime behave (that's why I didn't add it to javy::Config). Since we change the type that needs to be passed for initialize_runtime, I've bumped the javy-plugin-api version by a major version. We have minimal users of the plugin API crate so the impact should be limited.

Why am I making this change?

This should simplify the build and test infrastructure since we only need to build one artifact for the plugin and the CLI tests instead of two plugins. This should also help with cacheability of build artifacts. This will also make it easier to use the event loop functionality.

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.

@jeffcharles jeffcharles marked this pull request as ready for review November 22, 2024 20:16
@jeffcharles
Copy link
Collaborator Author

The Test CLI Features check will have to be marked as not required to merge this since this PR deletes that workflow.

Copy link
Member

@saulecabrera saulecabrera left a comment

Choose a reason for hiding this comment

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

Changes look good to me. Would it make sense to consider renaming the the option to allow-event-loop or something along those lines? It'd be ideal to signal that it's no longer considered experimental, for the same reasons exposed in: #833

@jeffcharles jeffcharles merged commit 71dabb5 into main Nov 26, 2024
6 checks passed
@jeffcharles jeffcharles deleted the jc.stop-using-event-loop-as-feature branch November 26, 2024 21:52
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