LouisCharlesC / safe

Header only read/write wrapper for mutexes and locks.
MIT License
147 stars 11 forks source link

Integrate GitHub actions and build system updates #25

Closed ahoarau closed 2 years ago

ahoarau commented 2 years ago

Open questions:

LouisCharlesC commented 2 years ago
remove doxygen

Whatever.

Remove travis and appveyor
Integrate Github actions

Very cool.

Switch testing to catch2 via fetchcontent

But I like doctest :(

Open questions:

Is coverage used ?

No idea. Maybe it was once, I absolutely forgot.

Is Warnings.cmake necessary ?

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 ?

ahoarau commented 2 years ago

No idea. Maybe it was once, I absolutely forgot.

To be removed then

Is Warnings.cmake necessary ?

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 ?

It's just for the tests that you enabled them. + I have no warnings on MSVC

LouisCharlesC commented 2 years ago

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!

ahoarau commented 2 years ago

Ok everything is ready !

LouisCharlesC commented 2 years ago

I updated the badge in the readme.
Is this part of the readme alright ?

mkdir build
cd build
cmake ..
sudo cmake --build . --target install
LouisCharlesC commented 2 years ago

It would be neat to have a coverage badge ;)

ahoarau commented 2 years ago

It would be neat to have a coverage badge ;)

Don't know how to do that. We need to update the readme as well for the CI badges

ahoarau commented 2 years ago

I updated the badge in the readme.
Is this part of the readme alright ?

mkdir build
cd build
cmake ..
sudo cmake --build . --target install

I'll suggest something