Skip to content

merge libdeno::DenoIsolate into core::Isolate#3605

Merged
bartlomieju merged 15 commits into
denoland:masterfrom
bartlomieju:merge_libdeno
Jan 6, 2020
Merged

merge libdeno::DenoIsolate into core::Isolate#3605
bartlomieju merged 15 commits into
denoland:masterfrom
bartlomieju:merge_libdeno

Conversation

@bartlomieju

Copy link
Copy Markdown
Member

No description provided.

Comment thread core/isolate.rs Outdated
Comment thread core/isolate.rs
Comment thread core/isolate.rs Outdated
@bartlomieju
bartlomieju requested a review from ry January 5, 2020 22:56
@bartlomieju bartlomieju changed the title [wip] merge libdeno::DenoIsolate into core::Isolate merge libdeno::DenoIsolate into core::Isolate Jan 6, 2020
Comment thread core/libdeno.rs
/// behaves much like an Arc<[u8]>, although a PinnedBuf currently can't be
/// cloned.
#[allow(unused)]
pub struct PinnedBuf {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is it unused?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

backing_store field is unused, I'll move attribute

Comment thread core/libdeno.rs
Comment thread core/libdeno.rs
pub unsafe fn deno_snapshot_delete(s: &mut deno_snapshot) {
todo!()
}
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just remove this dead code

@ry
ry requested a review from piscisaureus January 6, 2020 14:52
Comment thread core/bindings.rs
Comment thread cli/worker.rs
pub struct Worker {
pub name: String,
isolate: Arc<Mutex<deno_core::Isolate>>,
isolate: Arc<Mutex<Box<deno_core::Isolate>>>,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@piscisaureus Are you ok with forcing users of deno_core to use Box<Isolate> ?

@ry ry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM - nice work - I'd like to have @piscisaureus give it a glance before merging.

@ry ry left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@bartlomieju
bartlomieju merged commit 870622d into denoland:master Jan 6, 2020
@bartlomieju
bartlomieju deleted the merge_libdeno branch January 6, 2020 15:25
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