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.

Sign up for GitHub

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

Jump to bottom

chore: mark op1 as deprecated and port most core ops #279

Merged
merged 3 commits into from
Oct 24, 2023
Merged

chore: mark op1 as deprecated and port most core ops #279

merged 3 commits into from
Oct 24, 2023

Conversation

mmastrac
Copy link
Contributor

@mmastrac mmastrac commented Oct 24, 2023 •
edited
Loading

Before we remove #[op], give embedders some notice to switch over to #[op2]. Note that I could not port the wasm ops yet -- we'll address that in #275

@mmastrac mmastrac force-pushed the op1_deprecation branch 3 times, most recently from 1fe9416 to ec22190 Compare October 24, 2023 21:39
mmastrac
mmastrac commented Oct 24, 2023
View reviewed changes
core/examples/hello_world.rs

// This is a hack to make the `#[op]` macro work with
// deno_core examples.
// You can remove this:
use deno_core::*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a more convenient import for most of the examples, so leaving it here (it's no longer required though)

mmastrac
mmastrac commented Oct 24, 2023
View reviewed changes
core/examples/http_bench_json_ops/main.rs Outdated
use deno_core::OpState;
use deno_core::Resource;
use deno_core::ResourceId;
use deno_core::*;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ditto

mmastrac
mmastrac commented Oct 24, 2023
View reviewed changes
core/runtime/tests/ops.rs
@@ -288,42 +289,6 @@ fn test_op_detached_buffer() {
.unwrap();
}

#[test]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unused feature, just removing for now

mmastrac
mmastrac commented Oct 24, 2023
View reviewed changes
core/runtime/tests/snapshot.rs
@@ -1,4 +1,5 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
use crate as deno_core;
Copy link
Contributor Author

@mmastrac mmastrac Oct 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I might remove the op2(core) and just do this in the rare files in deno_core that have ops. core adds quite a bit of noise to the internal ops and it really isn't required.

Copy link
Member

Choose a reason for hiding this comment

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

I like this idea

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll follow up with it in a later pass!

bartlomieju
bartlomieju reviewed Oct 24, 2023
View reviewed changes
core/examples/http_bench_json_ops/main.rs Outdated
Copy link
Member

Choose a reason for hiding this comment

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

We can remove this file altogether I think. It's named http_bench_json_ops - it has nothing to do with JSON ops that we did in 2020 or 2021 :D

@mmastrac mmastrac enabled auto-merge (squash) October 24, 2023 22:57
@mmastrac mmastrac merged commit 73a90d3 into denoland:main Oct 24, 2023
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 6, 2023
@kylewillmon
Bump deno version
1473fd8 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).
@kylewillmon kylewillmon mentioned this pull request Nov 6, 2023
Merged
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 6, 2023
@kylewillmon
Bump deno version
d2c6682 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 13, 2023
@kylewillmon
Bump deno version
149928f 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 30, 2023
@kylewillmon
Bump deno version
150efb4 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).

The deno runtime version now comes directly from the deno snapshot. For
our usage, that means that the `Deno.version.deno` and `Deno.version.v8`
values are now empty strings where they were previously the Phylum
version and v8 version respectively.
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 30, 2023
@kylewillmon
Bump deno version
ebf1851 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).

The deno runtime version now comes directly from the deno snapshot. For
our usage, that means that the `Deno.version.deno` and `Deno.version.v8`
values are now empty strings where they were previously the Phylum
version and v8 version respectively.
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 30, 2023
@kylewillmon
Bump deno version
6fd3a8f 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).

The deno runtime version now comes directly from the deno snapshot. For
our usage, that means that the `Deno.version.deno` and `Deno.version.v8`
values are now empty strings where they were previously the Phylum
version and v8 version respectively.
kylewillmon added a commit to phylum-dev/cli that referenced this pull request Nov 30, 2023
@kylewillmon
Bump deno version (#1276)
ae113fa 
This includes migration to the new `op2` macro because `op` is now
deprecated (See denoland/deno_core#279). The new macro requires explicit
annotation for string and serde parameters and return values because of
the performance hit. We use these heavily (and don't care about the
performance).

The deno runtime version now comes directly from the deno snapshot. For
our usage, that means that the `Deno.version.deno` and `Deno.version.v8`
values are now empty strings where they were previously the Phylum
version and v8 version respectively.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@mmastrac @bartlomieju