src: add missing NODE_WANT_INTERNALS guards - #22514
Conversation
Sorry, something went wrong.
We generally add these to all headers that are considered internal to Node. These aren’t distributed as part of the headers tarball, so I think this does not have to be semver-major (and we have been changing the APIs in these headers freely anyway).
|
@addaleax build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/666/pipeline |
Sorry, something went wrong.
Sorry, something went wrong.
|
just wondering. Is it possible to add this macro to v8-inspector.h as well inside v8 folder or there is only one option to land something to V8 by itself and then backport it to Node 11? |
Sorry, something went wrong.
|
@ak239 I think that would be a question for @nodejs/v8-update … we don’t usually modify V8’s source files if we can avoid it, though. Would it help if we removed v8-inspector{,-protocol}.h from the headers tarball that we publish alongside with Node? |
Sorry, something went wrong.
|
Nice idea, it should help. As long as these headers are not available for native modules and we do not need to maintain ABI compatibility for them it will work. I will try to upload PR for this soon. Thanks! |
Sorry, something went wrong.
We generally add these to all headers that are considered internal to Node. These aren’t distributed as part of the headers tarball, so I think this does not have to be semver-major (and we have been changing the APIs in these headers freely anyway). PR-URL: #22514 Reviewed-By: James M Snell <jasnell@gmail.com>
We generally add these to all headers that are considered internal to Node. These aren’t distributed as part of the headers tarball, so I think this does not have to be semver-major (and we have been changing the APIs in these headers freely anyway). PR-URL: #22514 Reviewed-By: James M Snell <jasnell@gmail.com>
We generally add these to all headers that are considered internal to Node. These aren’t distributed as part of the headers tarball, so I think this does not have to be semver-major (and we have been changing the APIs in these headers freely anyway). PR-URL: #22514 Reviewed-By: James M Snell <jasnell@gmail.com>
We generally add these to all headers that are considered internal to Node. These aren’t distributed as part of the headers tarball, so I think this does not have to be semver-major (and we have been changing the APIs in these headers freely anyway). PR-URL: #22514 Reviewed-By: James M Snell <jasnell@gmail.com>
We generally add these to all headers that are considered
internal to Node.
These aren’t distributed as part of the headers tarball,
so I think this does not have to be semver-major
(and we have been changing the APIs in these headers
freely anyway).
Checklist