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

Standalone function refactor #44

Closed GretaCB closed 7 years ago

GretaCB commented 7 years ago

Per https://github.com/mapbox/node-cpp-skel/issues/10 and due to the ever-expanding uses-cases and complexity within the bench branch, there's a clear need to simplify the skel.

This PR is the start of refactoring the skel to allow for less interconnected code, making it easier to scale and to add new and separate implementations. Each new level of complexity can have its own .cpp and .hpp files within /src/your_code_here/, and pull them all together via module.cpp

That way, if you fork node-cpp_skel, we can document what you'd need to do:

Next

cc @springmeyer @mapsam

codecov-io commented 7 years ago

Codecov Report

Merging #44 into master will increase coverage by 0.04%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #44      +/-   ##
==========================================
+ Coverage   98.82%   98.86%   +0.04%     
==========================================
  Files           2        4       +2     
  Lines          85       88       +3     
==========================================
+ Hits           84       87       +3     
  Misses          1        1
Impacted Files Coverage Δ
src/hello_world.hpp 0% <ø> (ø) :arrow_up:
src/hello_world.cpp 100% <ø> (ø) :arrow_up:
src/module.cpp 100% <100%> (ø)
src/standalone/hello.cpp 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 59d0092...10e3110. Read the comment docs.

GretaCB commented 7 years ago

Hey @springmeyer , just FYI, I integrated the HelloWorld class and re-added the original tests to retain them in master so that this merge is more of an addition than an entire sweep.

I noticed some funky behaviour in Travis that I'm curious about:

For some reason, the hello function is never added to the module. I removed ./src/hello_world.cpp from the build, so everything is working fine now, but curious why Travis was different.

GretaCB commented 7 years ago

Above ^^^ was due to being on node v0.10.x locally instead of node v4.