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

use async_hooks #29

Merged
merged 2 commits into from Feb 14, 2018
Merged

use async_hooks #29

merged 2 commits into from Feb 14, 2018

Conversation

mafintosh
Copy link
Owner

Uses async_hooks to track the handles and stacks. Much, much cleaner but only supported by node >= 8 AFAIK (cc @andreas_madsen).

Thoughts @ralphtheninja @yoshuawuyts ?

@ralphtheninja
Copy link
Contributor

@mafintosh I know that you usually want to have support for many node versions, but iirc there has been some problems with this module due to differences in node implementations. In this case, moving up to node 8 to make it work better and also cleaner code is a good trade off.

@ralphtheninja
Copy link
Contributor

New major I guess 😉

@yoshuawuyts
Copy link
Collaborator

@mafintosh cleaner, I like it

@mafintosh mafintosh merged commit 246063c into master Feb 14, 2018
@mafintosh mafintosh deleted the async_hooks branch February 14, 2018 16:34
@mafintosh
Copy link
Owner Author

2.0.0

@fluxsauce
Copy link
Contributor

fluxsauce commented Feb 22, 2018

This decision is not user friendly; the README doesn't state that Node 8 is required, there's no CHANGELOG or release notes, you don't have an engines in your package.json that would have highlighted the problem, and there's a lot of folks still on Node 6.

I get wanting to use the latest technologies. Please find a balance by updating documentation (state in the README the version requirements and what to do if you don't have it), enforcement (package.json engines), and using release notes to declare why it's a major version.

Example:

Node 8 and above:

npm i why-is-node-running

Earlier Node versions (no longer supported):

npm i why-is-node-running@v1.x

@mafintosh
Copy link
Owner Author

mafintosh commented Feb 22, 2018 via email

@fluxsauce
Copy link
Contributor

Starred this three weeks ago, finally got around to it :-)

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.

None yet

4 participants