search

chrisdew / protobuf

Protocol Buffers for Node.JS
http://code.google.com/p/protobuf-for-node/
Apache License 2.0
234 stars 71 forks source link

Support for node 0.11.x #43

Closed imkira closed 10 years ago

imkira commented 10 years ago

As the title mentions, this pull request is my first attempt to support node v0.11.x. I mainly used nan.h to give me the macros I needed to support node 0.8, 0.10, 0.11 without much effort. Just for reference, I was able to build this on v0.8.26, v0.10.22, and v0.11.13. Other versions are probably also supported, although nan.h didn't appear to go along with v0.11.12 that well. I ran npm test (node test/unittest.js) and everything succeeded, but I believe the current test base (test/unittest.js) is kind of thin, and we should have much much more tests.

I have another pending branch at https://github.com/chrisdew/protobuf/pull/42 but for convenience purposes I made a different branch for this one and I would be very happy if you could review the source and hopefully pull it in the main repository.

chrisdew commented 10 years ago

Thanks for this, also see: https://github.com/chrisdew/protobuf/issues/41

imkira commented 10 years ago

Yeah. Thanks. Since we don't have that many tests, I tried to be less disruptive and keep changes to a minimum comparatively to #41 which renames files, classes and changes the overall structure. The same pullreq also mentions the code segfault'ing with npm test which is the second reason why I decided to try it myself too.

chrisdew commented 10 years ago

I tested this with my application on node v0.10.25 and everything was happy.

chrisdew commented 10 years ago

Thanks for your work on this. A new version (0.11.0) has just been published.

imkira commented 10 years ago

Thanks a lot!

mikepb commented 10 years ago

:+1: I get ocd about refactoring!