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

util: make inspect more reliable #4098

Merged
merged 1 commit into from
Dec 2, 2015
Merged

Conversation

evanlucas
Copy link
Contributor

34a3591 added pretty printing for
TypedArray, ArrayBuffer, and DataView. This change allows inspecting
those across different contexts.

Since instanceof does not work across contexts, we can use
v8::Value::IsTypedArray, v8::Value::IsArrayBuffer, and
v8::Value::IsDataView

/cc @bnoordhuis

@evanlucas evanlucas added the util Issues and PRs related to the built-in util module. label Dec 1, 2015
@evanlucas
Copy link
Contributor Author

@cjihrig
Copy link
Contributor

cjihrig commented Dec 1, 2015

LGTM if the CI is happy.

@jasnell
Copy link
Member

jasnell commented Dec 1, 2015

LGTM

@evanlucas
Copy link
Contributor Author

Will land tomorrow if there are no objections. Thanks!

@bnoordhuis
Copy link
Member

LGTM

34a3591 added pretty printing for
TypedArray, ArrayBuffer, and DataView. This change allows inspecting
those across different contexts.

Since instanceof does not work across contexts, we can use
v8::Value::IsTypedArray, v8::Value::IsArrayBuffer, and
v8::Value::IsDataView

PR-URL: nodejs#4098
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
@evanlucas evanlucas closed this Dec 2, 2015
@evanlucas evanlucas deleted the utiltypedarray branch December 2, 2015 12:13
@evanlucas evanlucas merged commit 24012a8 into nodejs:master Dec 2, 2015
@evanlucas
Copy link
Contributor Author

landed in 24012a8. Thanks!

@rvagg
Copy link
Member

rvagg commented Dec 5, 2015

I'm going to add semver-major because its dependency, 34a3591 / #3793, is tagged as semver-major and we need to keep them both off v5.x, I'm also going to remove lts-watch-v4.x.

I'm not convinced that 34a3591 should be semver-major but that's a discussion for #3793.

/cc @bnoordhuis

@rvagg rvagg added semver-major PRs that contain breaking changes and should be released in the next major version. and removed lts-watch-v4.x labels Dec 5, 2015
@rvagg
Copy link
Member

rvagg commented Dec 5, 2015

Note on semver-major: I think a case could be made that this is semver-major on its own, but I'd really just like the clarity that this is tied to another semver-major and therefore needs to stay out of backports, we could possibly remove the tag before pushing 6.0.0 out if we want to keep that out of the release notes. Perhaps another label would be helpful for these semver edge-cases? Same applies to the (node:pid) logging changes, there's a bunch of them together with only the first one being labelled semver-major.

@jasnell jasnell mentioned this pull request Mar 17, 2016
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
34a3591 added pretty printing for
TypedArray, ArrayBuffer, and DataView. This change allows inspecting
those across different contexts.

Since instanceof does not work across contexts, we can use
v8::Value::IsTypedArray, v8::Value::IsArrayBuffer, and
v8::Value::IsDataView

PR-URL: nodejs#4098
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-major PRs that contain breaking changes and should be released in the next major version. util Issues and PRs related to the built-in util module.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants