nodejs / llnode

An lldb plugin for Node.js and V8, which enables inspection of JavaScript states for insights into Node.js processes and their core dumps.
Other
1.15k stars 99 forks source link

build: add node-gyp as a dependency #357

Closed mmarchini closed 4 years ago

mmarchini commented 4 years ago

Fixes: https://github.com/nodejs/llnode/issues/355

Signed-off-by: Matheus Marchini mmarchini@netflix.com

bnoordhuis commented 4 years ago

As a maintainer of node-gyp, depending on a specific version of node-gyp is something I advise against. You should normally use the one that's bundled with node+npm.

mmarchini commented 4 years ago

Thanks for the feedback @bnoordhuis, I'll probably not merge this then.

Any suggestions on #355? Or is it a situation we just can't handle in code, since it's caused by Ubunut/Debian distributing a non-official Node.js package?

bnoordhuis commented 4 years ago

355 seems like a problem of debian or ubuntu's own making. If those distros tracked v8.x LTS like they should have, they'd be at a node-gyp version that's new enough (v5.0.5 vs v3.6.2.)

mmarchini commented 4 years ago

Closing in favor of https://github.com/nodejs/llnode/pull/367

coveralls commented 6 months ago

Pull Request Test Coverage Report for Build ceaaa9333cac9c4a46c36c0c8d4ef286e7086f1b-PR-357

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report. To ensure accuracy in future PRs, please see these guidelines. A quick fix for this PR: rebase it; your next report should be accurate.


Files with Coverage Reduction New Missed Lines %
src/constants.cc 1 80.3%
src/error.h 1 87.5%
src/llv8-constants.cc 2 85.2%
src/llv8.cc 3 71.04%
test/common.js 7 80.08%
src/llnode_api.cc 8 87.5%
src/node-inl.h 11 0.0%
src/node.h 12 20.0%
test/plugin/workqueue-test.js 13 55.17%
src/node.cc 21 37.5%
<!-- Total: 215 -->
Totals Coverage Status
Change from base Build 7b9598e9dad0a00e1b7ddd1aabd9cf7dddb3cab4: 3.8%
Covered Lines: 3715
Relevant Lines: 4711

💛 - Coveralls