-
Notifications
You must be signed in to change notification settings - Fork 388
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
Feature/extend build dependency #80
Feature/extend build dependency #80
Conversation
the header only fmt lib is used to show this
doctest_discover_tests() is only availabe if doctest is not imported with find_packag()
Codecov Report
@@ Coverage Diff @@
## master #80 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 2 2
Lines 12 12
=========================================
Hits 12 12
Continue to review full report at Codecov.
|
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.
Hey, thanks for the PR! I do like the idea of adding an example C++ dependency to the main library. Some points should be addressed though, as I'd like to keep the starter as simple and general as possible. Note that a lot is personal preference, which is why we encourage users to create their own forks with their personal modifications.
respect most review comments
no default cxx language settings install fmt link against fmt::fmt export dependency too if not needed ...
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
update to current PackageProject version 1.5.0 too
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.
Changes looking good, thanks! Some small things remain.
Co-authored-by: Lars Melchior <TheLartians@users.noreply.github.com>
remove NOLINT comment
needs to use doctest cmake modules ... it is not my idea!
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.
Awesome, thanks for the changes! I think having an example dependency in the main CMakeLists is a great improvement for the starter!
add build time dependency to static lib Greeter
this also prevents this #79 issue