Take module version from node headers. #109
Conversation
| @@ -6,4 +6,7 @@ extern "system" { | |||
| #[link_name = "NeonSys_Module_ExecKernel"] | |||
| pub fn exec_kernel(kernel: *mut c_void, callback: extern fn(*mut c_void, *mut c_void, *mut c_void), exports: Local, scope: *mut c_void); | |||
|
|
|||
| #[link_name = "NeonSys_Module_GetVersion"] | |||
| pub fn get_version() -> u32; | |||
There was a problem hiding this comment.
Oops. Wrong type. Should be i32 to match __NODE_MODULE
59c9916 to
a74fcbf
Compare
|
Sorry - new to Rust. Worked out the macro_internal stuff, so it all works properly now. |
If module abi version isn't specified in env, take it from the included node headers.
a74fcbf to
51bd3cd
Compare
|
Helps if I use the right account too. >< |
|
Can you also touch the invocation to not pass the environment variable anymore? This will make the |
|
Superseded by: #123 |
|
This is awesome, and I'm sorry for taking so long to review! I'm working with @jedireza to try to keep on top of PRs more regularly. Because this needs to sync with neon-bindings/neon-cli#31 it's worth merging independently of #123, and I think some aspects of #123 won't be able to land as-is anyway. So I think we should merge this one. As far as I can tell, this improvement has the same behavior as the way it originally works, but is simply less brittle. So it seems like a clear win. Thanks so much! |
|
(I think @jedireza wanted to just test this out in a few versions of Node before merging, so I'll wait to merge till he's ready.) |
|
r+ I tested using the latest node 4, 6, 7 with mac and linux. |
|
(Next I'll merge the commit that eliminates the env variable entirely.) |
If module abi version isn't specified in env, take it from the included node headers.