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

Next #334

Closed
wants to merge 84 commits into from
Closed

Next #334

wants to merge 84 commits into from

Conversation

kkoopa
Copy link
Collaborator

@kkoopa kkoopa commented May 7, 2015

Start collecting pull requests for NAN 2 here.

@kkoopa kkoopa added this to the 2.0 milestone May 7, 2015
@bnoordhuis
Copy link
Member

LGTM although the CI seems (partially) unhappy.

Out of curiosity, why was NanNewContextHandle deprecated?

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 8, 2015

AppVeyor hasn't pulled in build images for io.js 2 yet, they have it scheduled
for today, so it should start working by itself soon enough.

NanNewContextHandle was replaced with NanNew.
On Friday 08 May 2015 03:14:21 Ben Noordhuis wrote:

LGTM although the CI seems (partially) unhappy.

Out of curiosity, why was NanNewContextHandle deprecated?


Reply to this email directly or view it on GitHub:
#334 (comment)

@kkoopa kkoopa force-pushed the next branch 6 times, most recently from e931e21 to b1f2c47 Compare May 14, 2015 13:49
@kkoopa kkoopa force-pushed the next branch 4 times, most recently from 7146bc3 to 6b41ba7 Compare May 20, 2015 09:17
@kkoopa
Copy link
Collaborator Author

kkoopa commented May 22, 2015

Only things missing now are:

  • the new Buffer implementation
  • sticking everything (but macros) in namespace Nan
  • replacement of some NanNew<String> routines that should not be

@kkoopa kkoopa mentioned this pull request May 22, 2015
@kkoopa
Copy link
Collaborator Author

kkoopa commented May 22, 2015

Don't know why appveyor lists a failure although nothing failed. Can't reproduce.

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 22, 2015

Probably fixed it now. It was due to a sort of broken test (more like broken io.js). Calling gc() is supposed to be synchronous, but it does not appear to be so really. Made a polling loop to circumvent it.

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 25, 2015

A whole lot of documentation will need to be written and rewritten. Should we make use of Doxygen or some other similar alternative?

It would be nice to have proper documentation that did not require too much prerequisite API knowledge. In the past, this was less of a problem, as NAN was mostly used to port existing addons, but now almost all of them use NAN and the docs are more important for new developments.

Should NAN be documented the same way a core module would be documented, to fit in with other node docs?

P.S. I don't like writing documentation.

@agnat
Copy link
Collaborator

agnat commented May 26, 2015

I'm not a big fan of doxygen. It is either in the way or annoyingly uninformative. Sometimes both. I'd rather help with writing proper documentation. IMO it is the only way to describe things like NanNew<>() for the average reader.

P.S. I don't either ... but then who does?

@kkoopa
Copy link
Collaborator Author

kkoopa commented May 26, 2015

I'm not that big a fan of Doxygen myself either. I like reading its output, but don't like seeing it in the code or writing it. The reason I thought about it is because many classes are almost directly derived from V8. Please do help out with writing proper documentation, I have so many things to do that documentation gets rather low priority, even though I know that documentation is important for the end-user.

@kkoopa
Copy link
Collaborator Author

kkoopa commented Jul 31, 2015

Merged. 2.0.0 Is out.

@rvagg Nothing should block io.js 3.0 anymore.

@kkoopa kkoopa closed this Jul 31, 2015
@kkoopa kkoopa deleted the next branch July 31, 2015 11:57
@rvagg
Copy link
Member

rvagg commented Jul 31, 2015

Woohoo! Thanks @kkoopa, I'm working on release notes for v3 now. Should be out soon.

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.

5 participants