-
Notifications
You must be signed in to change notification settings - Fork 285
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
Moving to nan #135
Comments
It looks like memwatch |
Other than memwatch it looks like the nan branch works with v0.10.29 of node. I wonder if we should either remove memwatch since it's only used in integration tests or we could use the pull request's git url which is discouraged. My vote is removing the memwatch dependency. |
@joeferner I'm down with removing it. |
|
@joeferner Can you take a look at this? https://github.com/joeferner/node-java/blob/use-nan/src/methodCallBaton.cpp#L67 I have a feeling the scope is getting cleared when the callback is called, thus erasing the value. This only happens on |
After switching MethodCallBaton to use NanAsyncWorker it looks like it works on 0.11 and 0.10 for me. |
Are we ready to merge the use-nan branch? |
Looks like we are with a passing build! |
@joeferner |
Awesome this is really great. @jsdevel thanks for all your help. |
My pleasure. I feel like I learned a lot about v8 so I appreciate the opportunity! |
@joeferner I rebased the use-nan branch so your changes now appear on top of the latest. Let me know if this causes any issues for you.
The text was updated successfully, but these errors were encountered: