Standardize build location

[mapsam]

Any binaries built should follow a standard pattern, even for header-only libraries. Let's output the test binaries in build/Debug and build/Release directories.

cc @springmeyer @GretaCB

[springmeyer]

👍 You could do this by modifying the Makefile (Make of course is bare bones and it is up to you where to put your binaries). Or you could feed 2 birds with one hand and move this project to cmake or gyp which both control where the binaries go and default to build/Release, etc.

My slight preference is to move to cmake since this is something that would help projects, based on hpp-skel, scale as they grow. In short: I feel that hpp-skel is simple enough that Make is an elegant solution. But I presume that 100% of projects that use hpp-skel will grow in complexity eventually (will have muliple .cpp files and multiple unit tests perhaps) and therefore will benefit from cmake. You can use https://github.com/mapbox/spatial-algorithms/blob/master/CMakeLists.txt as an example to get started.

[mapsam]

@springmeyer considering this is the skel, do you think it'd be worth having both a Makefile and cmake for example's purpose?

[springmeyer]

worth having both a Makefile and cmake for example's purpose?

👍 This is a good idea in the sense that C++ devs are often have opinionated preferences. Providing both would help them move quickly. Always easy to delete the one they don't want as long as each example is self contained.

[mapsam]

Another thought here @springmeyer - is having a Release mode necessary for a header-only library? It feels like the travis file could be two matrices, one for OSX and one for Linux and use the flags defined in the Makefile/cmake.

The only thing BUILDTYPE=Release does different is the optimization flag -O3 versus -O0 (zero), which is less important for a single test build (maybe?).

[springmeyer]

Great points. If the goal is reducing complexity and keeping only the essential bits, then only building the unit tests in Debug is a good idea.

However I feel like the goal of providing a solid structure for future development demands keeping Release. The reasons to support both (and test both on travis) for projects based on hpp-skel are:

• Providing basic benchmarks scripts (like phase 1 at https://github.com/mapbox/node-cpp-skel/pull/29#issuecomment-319533657) would be ideal and benchmarks need to be compiled in Release mode
• Rarely, but it can happen, bugs might manifest only in Release mode or Debug mode so running the unit tests in both modes is ideal.

[mapsam]

Ah, great points @springmeyer! I didn't think about the benchmarking that release provides.

goal of providing a solid structure for future development demands keeping Release

💯 agree with the bullets you mention above - thanks for providing a bit more context.

[springmeyer]

Thought a little more about this @mapsam. While I still definitely like the plan of having and testing both Release and Debug mode (on travis), there is one side effect worth digging into. It is that:

• Testing both Debug and Release doubles the matrix size. This can be annoying when jobs are backed up (common for OS X)
• So the tendency, accidental or not, is to end up without full coverage of Release or Debug modes when other variables require extra jobs in the matrix.

On the latter point of missing coverage, this might matter: what if a bug only manifests in Debug mode on OS X, but you don't have a job that tests it, and the bug gets to production on linux and 💥 . It could have been prevented: you had the unit test, the bug just did not trigger because you missed testing the build mode that happened to surface it. Ugh.

A real world example of missing coverage is https://github.com/mapbox/vector-tile/blob/master/.travis.yml#L5-L49. Because we want to also test more compilers, and that would explode the matrix, we are missing coverage of Debug mode with certain compilers. Not a great thing.

So, ideally we can keep the matrix smallish and have more full coverage of build modes. To do this we could have the hpp-skel test both Release and Debug mode for each matrix by removing it as a matrix option. Like https://github.com/mapbox/wagyu/commit/0378117ae6a8d53d19fe1d99ff71a3196b5470e9.

[springmeyer]

Or you could feed 2 birds with one hand and move this project to cmake or gyp which both control where the binaries go and default to build/Release, etc.

Turns out I've had a longstanding misunderstanding about cmake. I thought that cmake automatically versioned in build/Release/some_executable or build/Debug/some_executable. But it turns out it does not (as seen in #24). By default binaries just go into ./build. This is nice because it gives a predicable place to call unit tests. So I stand corrected.

So the improvement for free in moving to cmake is getting out of source builds but not necessarily a versioned path to those builds. We can consider adding that in the future, but I don't think it is essential now.

Overal, I think this ticket is ready to be closed. Over in #24 @GretaCB has now ported us to use cmake. In that branch we are supporting both Debug and Release builds of the tests, per above. Also per above we are testing both Debug and Release builds on each travis matrix job to ensure the highest coverage.

[mapsam]

Thanks for the follow-up @springmeyer, and thanks to you both for running this and #24 out!