-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Integrate node-streams-aff
into library
#52
Conversation
I'll merge this for now, but if there are any other changes we should make, we can do it in a future PR. |
Great, thanks. I'll check this out later. |
@jamesdbrock Have you had a chance to review this? Since this repo hasn't had a new release yet, it's blocking the other work I've done. |
Yeah looks great. I see you've swapped in all the new EventEmitter stuff and you've got all the tests passing. |
The foreign imports in the tests is weird though, why was that necessary? Can't those foreign imports be imported instead from other purescript-node packages? |
Unfortunately, no, because all of those depend on |
Description of the change
Copies
node-streams-aff
and its tests into this library, excluding two APIs:newReadable
andpush
. I think that function could be better supported to allow for custom streams, but I didn't port that or implement my own version for two reasons:Readable
from aString
, which has been supported viafromString
.@jamesdbrock, can you take a look at this?
Checklist: