fix: building sonic boom safe #316
Conversation
.taprc
Outdated
| @@ -1,4 +1,5 @@ | |||
| ts: false | |||
| check-coverage: false | |||
There was a problem hiding this comment.
took it from pino, otherwise tests will fail if not reaches 100% coverage. https://github.com/pinojs/pino/blob/f8696ab5fb0e5a819b6255af420b29fb075cbdee/.taprc#L3
There was a problem hiding this comment.
Please add the istanbul ignores. Let's not lower the code coverage here.
lib/utils.js
Outdated
| } | ||
| return stream | ||
|
|
||
| /* istanbul ignore next */ |
There was a problem hiding this comment.
Why are there so many istanbul directives in this new code?
There was a problem hiding this comment.
This code block comes from pino as you can see here https://github.com/pinojs/pino/blob/f8696ab5fb0e5a819b6255af420b29fb075cbdee/lib/tools.js#L335-L373
I'm not sure if we need to cover this part as there is no unit test to cover it in pino.
.taprc
Outdated
| @@ -1,4 +1,5 @@ | |||
| ts: false | |||
| check-coverage: false | |||
There was a problem hiding this comment.
Please add the istanbul ignores. Let's not lower the code coverage here.
Pull Request Test Coverage Report for Build 1995385865
💛 - Coveralls |
|
There are likely a few more lines to skip to satisfy the coverage requirements in Node v12: https://github.com/pinojs/pino-pretty/runs/5573573041?check_suite_focus=true. |
Added the following block to build safe sonic boom.
Closes #304