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

use CPM.cmake module to fetch project_options #207

Closed
wants to merge 1 commit into from
Closed

use CPM.cmake module to fetch project_options #207

wants to merge 1 commit into from

Conversation

ClausKlein
Copy link
Contributor

@ClausKlein ClausKlein commented Mar 30, 2022

ENABLE_CONAN by default to test conan with CPM fetch contents
disable gui dependency for simlicity (and reduce buildtime)

see too aminya/project_options#116

ENABLE_CONAN by default to test conan with CPM fetch contents
disable gui dependency for simlicity (and reduce buildtime)
@@ -1,6 +1,6 @@
find_package(Catch2 REQUIRED)

include(CTest)
#XXX include(CTest)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

why do you include CTest modue?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you want to enable submissions to a CDash server?

# see https://github.com/cpm-cmake/CPM.cmake for more info
include(cmake/CPM.cmake)

# Add project_options v0.18.0, but my version! CK
Copy link
Contributor Author

Choose a reason for hiding this comment

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

only for test now, later original may be used!

@@ -92,6 +101,7 @@ dynamic_project_options(
PCH_HEADERS
<vector>
<string> # This is a list of headers to pre-compile, here are some common ones
ENABLE_CONAN
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am woundering conan was not enabled?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This is probably an artifact of the split between cpp_boilerplate_project and cpp_starter_project. Without ENABLE_CONAN here, this project cannot build at all.

@codecov-commenter
Copy link

Codecov Report

Merging #207 (d690b81) into main (2b9c3c9) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #207   +/-   ##
=======================================
  Coverage   80.00%   80.00%           
=======================================
  Files           3        3           
  Lines          30       30           
=======================================
  Hits           24       24           
  Misses          6        6           
Flag Coverage Δ
Linux ∅ <ø> (∅)
Windows 80.00% <ø> (ø)
macOS ∅ <ø> (∅)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b9c3c9...d690b81. Read the comment docs.

Copy link
Collaborator

@ddalcino ddalcino left a comment

Choose a reason for hiding this comment

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

I have to admit, I don't understand the purpose of this PR.

  • You are removing the sdl and fltk examples. Shouldn't you be starting from the upstream https://github.com/cpp-best-practices/cpp_boilerplate_project ? That project already has the examples removed.
  • CPM is certainly useful for many purposes, but I don't see why we need it here. FetchContent works just as well, and it doesn't rely on an external dependency. What are you trying to add?

@ClausKlein
Copy link
Contributor Author

This PR was only to tigger the ci test.
It shows the possibility to use CPM cmake module to fetch the project_options

@ClausKlein ClausKlein closed this Mar 31, 2022
@ddalcino
Copy link
Collaborator

This PR was only to tigger the ci test.

There are other ways to trigger a ci test. See GH actions syntax here: https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onevent_nametypes

I often add workflow_dispatch to my .github/workflows/ci.yml file, so I can manually trigger a workflow via a button in the Github UI. A lot of projects have their workflows set up to run workflows on push to any branch, not just main.

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.

3 participants