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

Add package config file generation for find_package #106

Open
wants to merge 1 commit into
base: cpp11
Choose a base branch
from

Conversation

a-pekar
Copy link

@a-pekar a-pekar commented Aug 23, 2019

(without package config version file)

@a-pekar a-pekar marked this pull request as ready for review August 23, 2019 09:47
@a-pekar a-pekar force-pushed the cpp11 branch 4 times, most recently from a2c41ba to 9f5ae9e Compare August 28, 2019 13:16
@a-pekar
Copy link
Author

a-pekar commented Aug 29, 2019

Please, review. Travis build seems to have failed for some irrelevant reason

@dascandy
Copy link
Owner

Travis build fails on Travis CI no longer being able to find the Clang-3.7 package for their majorly outdated Linux deployment - probably because it's gone out of LTS support a few years ago. I'll check if I can move it to more up to date Clangs and distros. No worries, none of your code was even looked at in the failed build, and all builds that were run are successful.

@@ -1,12 +1,24 @@

add_library(HippoMocks INTERFACE)
target_include_directories(HippoMocks INTERFACE .)
target_include_directories(HippoMocks INTERFACE $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> $<INSTALL_INTERFACE:include>)
if (NOT WIN32)
target_compile_options(HippoMocks
INTERFACE
-Wall -Wextra -pedantic -Wno-long-long -std=c++11
Copy link

Choose a reason for hiding this comment

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

This could lead to build failures if project is built once and used on different platforms (which is a possiblility given that it's header-only). Consider moving flags to HippoMocksTest project instead (where they are useful) to avoid forcing them on other people, and setting CXX_STANDARD + CXX_STANDARD_REQUIRED properties here instead of -std=c++11.

Copy link

Choose a reason for hiding this comment

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

This needs to be a cxx_std_11 compile feature since aforementioned properties aren't supported on interface targets :(

@a-pekar a-pekar force-pushed the cpp11 branch 4 times, most recently from 31a9bd3 to a2615f5 Compare August 30, 2019 09:53
- no package config version file
- moved compile options to test project
- added opportunity to skip tests
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.

4 participants