Skip to content
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

Remove fetching Catch2 and Fix #163 #164

Merged

Conversation

olivier-stasse
Copy link
Collaborator

Description

This PR fixes the fetching problem with Catch2.
It detects the Catch2 version and adapt the header inclusion.
If needed this can be refined in testing more precisely the version.
The CMakeLists.txt is simplified, and has a low impact in the code.

How I Tested

To test I added the rolling and the humble workflow to verify that both tests are feasible.

Do not merge before

  • link to dependent pull request

I fulfilled the following requirements

  • All new code is formatted according to our style guide (for C++ run clang-format, for Python, run flake8 and fix all warnings).
  • All new functions/classes are documented and existing documentation is updated according to changes.
  • No commented code from testing/debugging is kept (unless there is a good reason to keep it).

FetchContent_MakeAvailable(Catch2)
endif()
find_package(Catch2)
add_definitions(-DFOUND_CATCH2_MAJOR_VERSION=${Catch2_VERSION_MAJOR})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
add_definitions(-DFOUND_CATCH2_MAJOR_VERSION=${Catch2_VERSION_MAJOR})
if(Catch2_FOUND)
add_definitions(-DFOUND_CATCH2_MAJOR_VERSION=${Catch2_VERSION_MAJOR})
else()
include(FetchContent)
FetchContent_Declare(
Catch2
GIT_REPOSITORY https://github.com/catchorg/Catch2.git
GIT_TAG v3.6.0)
FetchContent_MakeAvailable(Catch2)
add_definitions(-DFOUND_CATCH2_MAJOR_VERSION=3)
endif()

I don't see the point in removing the fetch part

@olivier-stasse olivier-stasse merged commit f92e4b7 into open-dynamic-robot-initiative:master Jun 24, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants