Add Ja2Export target and get it to build on C++17 #88
Conversation
| @@ -0,0 +1,29 @@ | |||
| option(BUILD_JA2EXPORT "Build the Ja2Export tool." ON) | |||
There was a problem hiding this comment.
does this mean its by default built?
| @@ -0,0 +1,29 @@ | |||
| option(BUILD_JA2EXPORT "Build the Ja2Export tool." ON) | |||
|
|
|||
| if(BUILD_JA2EXPORT) | |||
There was a problem hiding this comment.
(I dont know best practices)
wouldnt it make more sense to skip the inclusion in the root CMakeLists instead of here?
would also make all what-to-build decisions be in a central location
There was a problem hiding this comment.
I like having the exporter tool decide whether to build configure itself or not. but there is a case for moving the option(...) to the root CMakeLists file.
|
the github workflow release would fail with this tool being built and the exe being put in the output dir next to the lang exes, as right now only one exe is expected is this exporter lang/app based? if not it probably makes sense to not build it at all for the lang/app builds and instead introduce another parallel build process and zip it as its own artifact |
|
the github workflow uses msbuild at the moment so it would, in fact, not fail with this change. :-) |
the msbuild workflow variant does not even build the tool ;) but the cmake workflow variant would |
nice little tool
majcosta
force-pushed
the
pr_add_ja2export
branch
from
7f41f84 to
eba30ea
Compare
nice little tool