Header-only lib #14

Activity

1. springmeyer commented on Oct 16, 2017

   

   springmeyer

   on Oct 16, 2017

   Contributor

   More actions

   👍 low priority, but this ticket can serve as a reminder to circle back on this once the code is working. Basically we should investigate organizing the code such that code depending on other header-only libs is separate from the code that depends on node/nan, as a first step.
2. mapsam commented on Oct 23, 2017

   

   mapsam

   on Oct 23, 2017

   ContributorAuthor

   More actions

   One thing that this would enable is more detailed benchmarks - we could incorporate learnings from hpp-skel as well to test with Google Benchmark, plus running benchmarks in node.
3. springmeyer commented on Oct 23, 2017

   

   springmeyer

   on Oct 23, 2017

   Last edited by springmeyer

   Contributor

   More actions

   Yes. Breaking apart the pure C++ code now, even while stored inside this repo, would enable easier direct unit testing in C++. @mapsam happy to walk you through what this looks like. In short https://github.com/mapbox/route-annotator/blob/master/binding.gyp is a good example. What that does is:

   • Takes all pure C++ code, compiles it into a static library (for best compiling speed. Note: ideally that static library is just the inclusion of the _ipl.hpp per .ipp implementation architecture hpp-skel#3)
   • Links the node module to that static lib
   • Builds unit tests as another targer
   • For ease of running everything, modifies the Makefile to test both the C++ unit tests and the node module at once: https://github.com/mapbox/route-annotator/blob/master/Makefile#L47
4. springmeyer commented on Feb 6, 2018

   

   springmeyer

   on Feb 6, 2018

   Contributor

   More actions

   Back here to share further thoughts and ideas for next actions. I think the main goal here should be to first separate the code that deals with vector tiles as much as possible from the code that deals with node specific functionality. After that split is in place then it will become more feasible/clear for whether the vector tile specific code can/should live elsewhere. Also, after that split it would become more feasible to add an emscriptem/web-assembly API interface inside vtquery to complement the node interface.

   So, what this would look like is:

   • A new .hpp file that consolidates code that does not depend on #include <nan.h>. The src/util.hpp would function as this if the CallbackError code where moved out.
   • The vtquery.cpp getting smaller as a lot of code moves out. For example most of the code inside Execute ideally could be moved into the non-nan header. The gocha here is that code depends on TileObject which has a small dependency on nan/node due to Nan::Persistent<v8::Object> buffer_ref - perhaps this could be templated or otherwise moved off the class to avoid this TileObject having a dependency on node? If it could then the TileObject could be used from a web assembly API...
5. springmeyer commented on Feb 6, 2018

   

   springmeyer

   on Feb 6, 2018

   Last edited by springmeyer

   Contributor

   More actions

   would enable easier direct unit testing in C++

   One update on this idea (seen above in the motivations). I no longer think that it is critical that we do any direct unit testing here in pure C++ because:

   • most of the code we would have wanted to directly unit test is now inside https://github.com/mapbox/vector-tile and we should unit test there
   • this repo is now at 98% coverage, so we're solid on coverage just from JS tests.
6. springmeyer commented on Feb 10, 2018

   

   springmeyer

   on Feb 10, 2018

   Contributor

   More actions

   Example of what I have in mind: mapbox/vtvalidate#14