fpagliughi / sockpp

Modern C++ socket library.
BSD 3-Clause "New" or "Revised" License
792 stars 126 forks source link

Fixed building for catch2 (>=3.0) #84

Closed kotopesutility closed 1 year ago

kotopesutility commented 1 year ago

Fixed building for new catch2 (>=3.0). I don't like my solution for CMake but I wanted to save building for old catch2 (<3.0) for Fedora or something like that.

fpagliughi commented 1 year ago

Oh, cool. I was thinking I should update the Catch2 support, but was a bit too busy to get to it.

I wasn't thinking of maintaining support for the older (2.x) version, but if it's this easy then maybe, why not?

But I wonder if it might be better to isolate the conditional compilation to a single, new test header file rather than leak it into all the test sources? Maybe make a tests/unit/catch2_version.h with the details:

#ifdef CATCH2_V2
    #include "catch2/catch.hpp"
#else
    #include "catch2/catch_all.hpp"
#endif

And the defined macro could be a little more descriptive, like CATCH_V2 or CATCH_V2X or something.

Either that, or say nevermind to maintaining v2, and just bump the requirement to v3.

kotopesutility commented 1 year ago

Well, I have force-pushed my solution. It includes catch2v2 support for distributions like Fedora, Debian, and so on.

I wasn't thinking of maintaining support for the older (2.x) version, but if it's this easy then maybe, why not?

Cool, let it be: CATCH2_V2 like in your example^

And the defined macro could be a little more descriptive, like CATCH_V2 or CATCH_V2X or something.

fpagliughi commented 1 year ago

Cool. That looks good. I may not have time to test it until the weekend, but assuming it all works as expected, I'll get it merged ASAP.

Thanks for the contribution!

kotopesutility commented 1 year ago

Any updates?)

fpagliughi commented 1 year ago

It seems to work for both v2 and v3. Nice.

I may get rid of the CMake warning for v2. If we support both versions (for now) then no need to complain about it. At least until we decide and state that v2 is obsolete and won't be maintained going forward.

Thanks for the help!