auto-register cpp-module in JS #23
Conversation
source/node-lib-qt-rss/main.cpp
Outdated
|
|
||
| node::lib::Initialize(); | ||
| node::lib::RegisterModule("CppDemoModule", { | ||
| node::lib::RegisterModule("cppDemoModule", { |
There was a problem hiding this comment.
If you're at it, maybe rename the module to cpp-demo-module? (see https://www.npmjs.com/package/module-best-practices#naming-conventions). This would obviously break the auto_bind option. Maybe convert cpp-demo-module into cppDemoModule inside the RegisterModule function automatically?
Just a thought, though.
There was a problem hiding this comment.
Maybe convert
cpp-demo-moduleintocppDemoModuleinside theRegisterModulefunction automatically?
I think converting the module-name inside of RegisterModule() is a bad idea, since it likely is not what the programmer consuming the API would expect. Instead, we should just change the JS code in rss_feed.js to comply with the name change.
I initially made the change you commented on so I didn't have to change all the JS-code, but with the bigger change this should be OK.
cmfcmf
left a comment
cmfcmf
left a comment
There was a problem hiding this comment.
Looks good now, feel free to merge after hpicgs/node#38
Only works with this pull request.