rmaxi-me commented 2 years ago

Description

Hey dear friend, I was bored and thought I'd improve some stuff! :smile:

Here is a recap, there are more details in my commit descriptions:

Renamed BUILD_TESTS to DYLIB_BUILD_TESTS to avoid name collisions
- Avoids name collisions for projects adding dylib as dependency
Improved compiler flags setup
- Check for compiler ID instead of the OS
Added "Treat warning as errors" cmake flag
- Optional flag to make the lib more consistent (set to ON in action workflow)
Moved DYLIB_BUILD_TESTS condition to make it easier for devs to contribute
- See 2b56e9e8 for more details (would be too long to copy paste here)
Added explicit dependency to dynamic_lib from unit tests
- Force test lib to build alongside unit tests
Replace empty string comparison with an is empty call
- Tiny perf improvement, but still :stuck_out_tongue:
Merged C++11 and C++17 workflows by adding a variable in the matrix
- Avoids repetitions, easier to modify, and you could even add more C++ version if you would like.

Changes include

[ ] Bugfix (non-breaking change that solves an issue)
[ ] New feature (non-breaking change that adds functionality)
[X] Breaking change (change that is not backwards-compatible and/or changes current functionality)

Breaking changes

Changed BUILD_TESTS to DYLIB_BUILD_TESTS in cmake.
Updated all references and the README.
But projects depending on this variable may need to update.

Even if not really needed, I think it's important to prefix variables with the project name to avoid collisions with other libs/projects.

Checklist

[ ] I have assigned this PR to myself
[ ] I have added at least 1 reviewer
[X] I have tested this code
[X] I have added sufficient documentation in the code

Additional comments

For more detailed explanations, please read my commit messages as to why I did those changes.

codecov[bot] commented 2 years ago

Codecov Report

Merging #61 (0af75bb) into main (0bdef6c) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main       #61   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           59        59           
  Branches         3         3           
=========================================
  Hits            59        59

Impacted Files	Coverage Δ
include/dylib.hpp	`100.00% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.