added fix for #181 but without any changes to test #183
Conversation
|
Hey, @dotnetCarpenter, sorry for the delay on this. I haven't forgotten about it, just haven't found time to review carefully. Since it's doing some interesting juggling with streams and child procs, I want to make sure I understand all the moving parts before merging, even if it seems to be working properly. |
|
@isaacs in short; only the services array is new and I wrapped line 244-260 in @evanlucas commit in a forEach and deleted some stuff that is now redundant or not needed. |
| @@ -59,8 +59,6 @@ var coverage = pipeToService | |||
| var coverageReport | |||
|
|
|||
| var nycBin = require.resolve('nyc/bin/nyc.js') | |||
| var coverallsBin = require.resolve('coveralls/bin/coveralls.js') | |||
There was a problem hiding this comment.
redundant, since the introduction of the services array
|
@isaacs I hope I answered your question with my last (updated) comment. If not, then please ask again, so I can clarify. |
|
Finally got around to landing this, with some updates so that it would work when running tap's tests with coverage turned on. (The double-wrap weirdness was making it fail strangely.) |
Hi @isaacs
Concluding that my first assumption is correct. Patch #182 is working as is