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

Use modern CMake #26

Closed
Flamefire opened this issue Aug 28, 2018 · 2 comments
Closed

Use modern CMake #26

Flamefire opened this issue Aug 28, 2018 · 2 comments

Comments

@Flamefire
Copy link

Just from your Readme:

target_link_libraries(${PROJECT_NAME} mos)
target_include_directories(${PROJECT_NAME} PUBLIC externals/mos/include)

The 2nd line should not be required if mos had setup its include dirs. Use target_include_directories and PUBLIC/PRIVATE/INTERFACE for all target_* functions. See "Effective CMake" by Daniel Pfeiffer,

@morganbengtsson
Copy link
Owner

Thanks, should be a bit cleaner now:
4a2efb8
95aa800
Quite sure I could read up on CMake a bit more though.

@Flamefire
Copy link
Author

Related: Is (e.g.) glm a public dependency or is it only used internally?
There is add_definitions(-DGLM_FORCE_RADIANS) at the top, if glm is public you should use target_compile_definitions(target PUBLIC ...) if not use PRIVATE and make the include directory PRIVATE too. Similar for others in target_include_directories: Decide and define what is only internally used and what is included in the external interface.
-DMOS_EFX can also be a PRIVATE in target_compile_definitions

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

No branches or pull requests

2 participants