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

Remove CoreResource::inspect_repr method #3274

Merged
merged 2 commits into from Nov 6, 2019

Conversation

@ry
Copy link
Collaborator

ry commented Nov 6, 2019

Towards simplifying (or better removing entirely) the CoreResource
trait. Resources should be any bit of privileged heap allocated memory
that needs to be referenced from JS, not very specific trait
implementations. Therefore CoreResource should be pushed towards being
as general as possible.
cc @bartlomieju
note: less code

ry added 2 commits Nov 6, 2019
Towards simplifying (or better removing entirely) the CoreResource
trait. Resources should be any bit of privileged heap allocated memory
that needs to be referenced from JS, not very specific trait
implementations. Therefore CoreResource should be pushed towards being
as general as possible.
fix
@ry ry requested a review from piscisaureus Nov 6, 2019
Copy link
Contributor

bartlomieju left a comment

LGTM

Towards simplifying (or better removing entirely) the CoreResource
trait. Resources should be any bit of privileged heap allocated memory
that needs to be referenced from JS, not very specific trait
implementations. Therefore CoreResource should be pushed towards being
as general as possible.

If we remove Resource trait then we won't be able to downcast to concrete type, so I agree that it should be minimal (minimal as in empty) but it shouldn't be removed entirely.

@ry ry merged commit 5c1deac into denoland:master Nov 6, 2019
10 checks passed
10 checks passed
test macOS-latest
Details
test_std macOS-latest
Details
test windows-2019
Details
test_std windows-2019
Details
test ubuntu-16.04
Details
test_debug ubuntu-16.04
Details
test_std ubuntu-16.04
Details
bench ubuntu-16.04
Details
lint ubuntu-16.04
Details
license/cla Contributor License Agreement is signed.
Details
@ry ry deleted the ry:resource_clean_up branch Nov 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.