docs: remove reference to Node 10 incompatibility #449
Conversation
The bug in V8 that meant we couldn't instrument Node 10 was fixed and Node 10.4.0 shipped with a newer version of V8. This is the original bug report in Node: nodejs/node#20516 This is the PR that fixed it in Node: nodejs/node#19989 Closes elastic#448
test/instrumentation/modules/hapi.js
Outdated
| @@ -13,9 +13,6 @@ var semver = require('semver') | |||
| // hapi 17+ requires Node.js 8.9.0 or higher | |||
| if (semver.lt(process.version, '8.9.0') && semver.gte(pkg.version, '17.0.0')) process.exit() | |||
|
|
|||
| // hapi does not work on Node.js 10 because of https://github.com/nodejs/node/issues/20516 | |||
| if (semver.gte(process.version, '10.0.0-rc')) process.exit() | |||
There was a problem hiding this comment.
Rather than fully removing this, can we change it to a range to indicate that certain early versions of 10.x are broken?
|
Looks like the tests are failing on 10.x. 🤔 |
|
I think it's "just" hapi behaving different under Node 10 for some reason. As far as I can see hapi is emitting some events now that it wasn't doing under Node 9 when running our tests. Those are caught as errors and that causes our tests to fail. At least that's what I get from looking at the first failure. The other failures might be different. I will have to look into if this is expected behavior and if so update the tests to expect it. This can take a while unfortunately. |
|
Do you want to perhaps just merge the docs change and split the test change out to another PR? |
Codecov Report
@@ Coverage Diff @@
## master #449 +/- ##
==========================================
- Coverage 82.27% 82.26% -0.01%
==========================================
Files 42 42
Lines 2268 2267 -1
==========================================
- Hits 1866 1865 -1
Misses 402 402
Continue to review full report at Codecov.
|
|
I'll merge this even though Jenkins is acting up. It's unrelated. |
The bug in V8 that meant we couldn't instrument Node 10 was fixed and Node 10.4.0 shipped with a newer version of V8.
This is the original bug report in Node: nodejs/node#20516
This is the PR that fixed it in Node: nodejs/node#19989
Closes #448