Skip to content
This repository has been archived by the owner on Apr 19, 2023. It is now read-only.

Update dependecies #46

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

StevenvdSchoot
Copy link

@StevenvdSchoot StevenvdSchoot commented Jan 17, 2023

Updated some dependecies:

  • Cmake: Support version 24 and 25
  • project_options: update to version 0.26.3
  • Catch2: upgrade to version 3 (3.3.0 specifically)
  • spdlog: update to version 1.11

@@ -28,10 +28,6 @@ find_package(Catch2 CONFIG REQUIRED)

include(Catch)

add_library(catch_main OBJECT catch_main.cpp)

Choose a reason for hiding this comment

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

I've kept this lines to avoid changing to Catch2::Catch2WithMain in all other target_link_libraries. It worked anyway.

Copy link
Author

Choose a reason for hiding this comment

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

Could you elaborate on what the benefits are of keeping the catch_main target? When keeping it it just serves as an alias for Catch2::CatchWithMain. My initial thought was to remove it, since it shortens and simplifies the cmake file and thus servers the purpose of a minimal starter project better.

Choose a reason for hiding this comment

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

I think none to be honest. But you see that's an project_options include. I'm not sure if it changes something or not.

@@ -28,10 +28,6 @@ find_package(Catch2 CONFIG REQUIRED)

include(Catch)

add_library(catch_main OBJECT catch_main.cpp)
target_link_libraries(catch_main PUBLIC Catch2::Catch2)

Choose a reason for hiding this comment

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

Changed only here to target_link_libraries(catch_main PUBLIC Catch2::Catch2WithMain)

@viniciusferrao
Copy link

Can anyone with commit permission take a look on this? It's really useful.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants