-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Use gotham-like state for ops #7385
Conversation
Looks good generally. The comment on the "sync op fn" signature is the only serious comment. Also before diving deep into CLI I'd try to figure out the CLI metrics wrapper, or this could turn into a really sour experience in a couple (dozen) hours. |
@piscisaureus Further edits to the interface defined in core. Exposing json_op_sync and json_op_async wrappers rather than having special op registration functions for json ops. OpTable moved to JsRuntime state. deno_core is green, CLI remains unintegrated. |
Seems everything is working but plugins... |
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.
LGTM, just a few minor nitpicks
merged it myself |
Provides a concrete state type that can be dynamically added to. This is necessary for op crates.
Rc<RefCell<OpState>>
OpRegistry
,OpRouter
traitsregister_op()
,register_op_json_sync()
,register_op_json_async()
back to be JsRuntime methodsget_error_class_fn
moved to OpStateStatus: deno_core tests/examples are good (
cargo test -p deno_core
). cli hasn't been integrated yet.