-
Notifications
You must be signed in to change notification settings - Fork 11
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
Integrate GitHub actions and build system updates #25
Conversation
Whatever.
Very cool.
But I like doctest :( Open questions:
No idea. Maybe it was once, I absolutely forgot.
I guess some people compile with warnings as errors, if an error stops their compilation because of a warning in safe, it's pretty bad, no ? |
To be removed then
It's just for the tests that you enabled them. + I have no warnings on MSVC |
Yeah, when I compile my tests, I should see any warning pop up as an error, which ensures no one will ever have a warning using safe. It's only on in the tests so it's not enforced to users. It's just extra burden for me and it's the reason you don't see any warnings in MSVC! |
remove coverage remove coverage cmake file
a01e820
to
6c200cf
Compare
having those set_target_properties( safe_tests PROPERTIES CXX_STANDARD 17 CXX_EXTENSIONS NO ) overrides anything you pass on cmdline.
Ok everything is ready !
|
I updated the badge in the readme.
|
It would be neat to have a coverage badge ;) |
@@ -2,17 +2,23 @@ cmake_minimum_required(VERSION 3.9.2) | |||
|
|||
cmake_policy(SET CMP0048 NEW) | |||
project(safe VERSION 1.0.0 LANGUAGES CXX) | |||
option(BUILD_TESTING "Build tests" ON) |
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.
Is this wanted ?
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.
Yes it's just to make it explicit.
The equivalent of the option + enable_testing is to simply include(CTest)
e6c4ef7
to
c8460cb
Compare
c8460cb
to
73a1d47
Compare
Don't know how to do that. We need to update the readme as well for the CI badges |
I'll suggest something |
makes it compatible with add_subdirectory and fetchcontent
b0596e3
to
22a7383
Compare
Open questions: