Skip to content
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

make node-speaker node10x compatible, use internal stream node module #120

Closed
wants to merge 1 commit into from

Conversation

hlolli
Copy link
Contributor

@hlolli hlolli commented Aug 30, 2018

This is actually no deadly necessary pull request. This mainly fixes bunch of deprecation warnings from Nan. I was in an attempt to fix a problem with new thread-workers in Node10 so I made these changes. But then found out that it wasn't the root of my problem nodejs/node#21481 otherwise the tests run, produces expected audio on my computer, all deps are bumped, with the readable dep removed in favor of the internal stream module provided by nodejs ecosystem. The old readable-stream module is mostly a polyfill for browser compatability as far as I understood it, bumping it to v3x caused errors with node-speaker.

Sorry about the noise, had to move this PR to another branch.

@hlolli
Copy link
Contributor Author

hlolli commented Aug 30, 2018

I bet my changes to mocha weren't fetched in CI tests? Some module cacheing I would guess.

@fwcd fwcd mentioned this pull request May 31, 2019
@LinusU
Copy link
Collaborator

LinusU commented Dec 7, 2019

Thanks for the PR, in the end we went with another approach here: #137. Sorry for not getting to this earlier ☺️

@LinusU LinusU closed this Dec 7, 2019
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.

2 participants