-
Notifications
You must be signed in to change notification settings - Fork 25
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[c++] General CMake clean-up #2762
Conversation
We are only using the compile options on C++ libraries, so no need to specify compile language. Using the generator expression for the target based compile options was causing a CMake error.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2762 +/- ##
==========================================
+ Coverage 89.87% 90.02% +0.15%
==========================================
Files 37 37
Lines 3939 3939
==========================================
+ Hits 3540 3546 +6
+ Misses 399 393 -6
Flags with carried forward coverage won't be shown. Click here to find out more.
|
The TileDB-SOMA library does not define a BUILD_COMMIT_HASH variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be functionally equivalent but uses better (newer?) cmake idioms -- a good change!
Take what I say with a grain of salt though as I am far from being cmake-fluent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
As noted on the other PR thread, the ❌ on R's CI run is unrelated.
* Add options explicitly to targets instead of to everything. Note: this means compile options are no longer propagated to third-party libraries included with `FetchContent` and `ExternalProject_Add`. * Move `find_package` calls and coverage flags to base `CMakeLists.txt` instead of adding them twice. * Re-arrange some CMake calls to reduce redundancy and improve readability. * Remove unused GIT_COMMIT_HASH compiler definition. * Change TileDB deprecation flag to use Core flag that is not over-written.
* Add options explicitly to targets instead of to everything. Note: this means compile options are no longer propagated to third-party libraries included with `FetchContent` and `ExternalProject_Add`. * Move `find_package` calls and coverage flags to base `CMakeLists.txt` instead of adding them twice. * Re-arrange some CMake calls to reduce redundancy and improve readability. * Remove unused GIT_COMMIT_HASH compiler definition. * Change TileDB deprecation flag to use Core flag that is not over-written. Co-authored-by: Julia Dark <24235303+jp-dark@users.noreply.github.com>
Issue and/or context: Closes #2664
Changes:
FetchContent
andExternalProject_Add
.find_package
calls and coverage flags to baseCMakeLists.txt
instead of adding them twice.