Skip to content

Conversation

@mistic
Copy link
Collaborator

@mistic mistic commented Dec 13, 2018

This is a revisit for #36
I just merged the branch present in the mentioned PR, applied the suggested review by @evilebottnawi

@jsf-clabot
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ mistic
❌ kevinburke

@alexander-akait
Copy link
Member

@mistic Just create new branch, copy/paste code and send a PR 👍

In the event that the read stream terminates before all of the data
has been read, the pipe will send an 'end' event. We should listen for
this event and hit the callback with it, instead of hanging forever.

Added tests to verify this. Verified the patch fixes the problem by
running the tests without the patch, observing that they timeout, then
running the tests with the patch and verifying you get an error back.
Attempting to run "npm install" on a new-ish Mac with Node v10.11
fails with a number of node-gyp compilation failures. Installing and
using the latest versions of node-sass, nan, and fsevents resolves
this failure, as each of those packages resolves the issues that led
to problems.

Update lodash to v4.17.11 to fix a security warning present in the
Travis CI configuration.

Add Node 10 to the build matrix so we can test for it.

I am nervous about the number of changes to package-lock.json and I am
not sure how to mitigate that - apparently NPM 6 changed the way
requirements are stored in the file so we'll just have to do this at
some point. npm/npm#20434
@codecov
Copy link

codecov bot commented Dec 13, 2018

Codecov Report

Merging #41 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@         Coverage Diff          @@
##           master   #41   +/-   ##
====================================
  Coverage       0%    0%           
====================================
  Files           7     7           
  Lines         372   384   +12     
  Branches       66    68    +2     
====================================
- Misses        325   335   +10     
- Partials       47    49    +2
Impacted Files Coverage Δ
src/readBuffer.js 0% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c3366d6...8aa538c. Read the comment docs.

@mistic mistic closed this Dec 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants