search

mapbox / node-cpp-skel

Skeleton for bindings to C++ libraries for Node.js using node-addon-api
Creative Commons Zero v1.0 Universal
72 stars 10 forks source link

add promise function #169

Closed mapsam closed 2 years ago

mapsam commented 2 years ago

Adds a promise function helloPromise to expose the Napi::Promise methods.

Lots of inspiration from https://github.com/nodejs/node-addon-examples/issues/85

cc @springmeyer @artemp

mapsam commented 2 years ago

Currently playing whack-a-mole with the clang TIDY options - here are two conflicting reports:

../src/standalone_promise/hello_promise.cpp:12:5: error: ‘standalone_promise::PromiseWorker::output’ should be initialized in the member initialization list [-Werror=effc++]
     PromiseWorker(Napi::Env const& env, std::string phrase, int multiply)

And if the change is made to add output() to the initializer list, the clang tidy build fails with:

/home/travis/build/mapbox/node-cpp-skel/build/../src/standalone_promise/hello_promise.cpp:17:11: error: initializer for member 'output' is redundant [readability-redundant-member-init,-warnings-as-errors]
          output() {}
artemp commented 2 years ago

Currently playing whack-a-mole with the clang TIDY options - here are two conflicting reports:

../src/standalone_promise/hello_promise.cpp:12:5: error: ‘standalone_promise::PromiseWorker::output’ should be initialized in the member initialization list [-Werror=effc++]
     PromiseWorker(Napi::Env const& env, std::string phrase, int multiply)

And if the change is made to add output() to the initializer list, the clang tidy build fails with:

/home/travis/build/mapbox/node-cpp-skel/build/../src/standalone_promise/hello_promise.cpp:17:11: error: initializer for member 'output' is redundant [readability-redundant-member-init,-warnings-as-errors]
          output() {}

Interestingly, I don't see this with make tidy (?)

mapsam commented 2 years ago

@artemp thanks for the review! Changes are in https://github.com/mapbox/node-cpp-skel/pull/169/commits/84c98487755da263f58be12092ad7dbf96dbb66f

mapsam commented 2 years ago

@artemp nope, accidentally committed those. Removed in https://github.com/mapbox/node-cpp-skel/pull/169/commits/19c4d4494ff446cbba4706e2b2a9ecdde270f951!

I'm working over at https://github.com/mapbox/node-cpp-skel/tree/xcode-updates to try and update the xcode versions, which appear to have relocated the location of std on my machine between versions, hence the commented-out clang++ settings. I'm just using the system default c++ to build now, which appears to work just fine. If you have thoughts on how to remedy this in node-pre-gyp, I'm all ears!

artemp commented 2 years ago

@artemp nope, accidentally committed those. Removed in 19c4d44!

I'm working over at https://github.com/mapbox/node-cpp-skel/tree/xcode-updates to try and update the xcode versions, which appear to have relocated the location of std on my machine between versions, hence the commented-out clang++ settings. I'm just using the system default c++ to build now, which appears to work just fine. If you have thoughts on how to remedy this in node-pre-gyp, I'm all ears!

Maybe try upgrading to clang-**-11 ?