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

DolphinQt2/CMakeLists: Use the target_* equivalents of add_definitions and include_directories #6598

Merged
merged 1 commit into from Apr 5, 2018

Conversation

lioncash
Copy link
Member

@lioncash lioncash commented Apr 5, 2018

add_definitions and include_directories don't operate on a by-target basis, they act on a by-directory basis (i.e. if we defined two targets, A and B, in this CMakeLists file, add_definitions would add the definitions to the COMPILE_DEFINITIONS directory property which both A and B would implicitly use).

The same idea applies to include_directories, only it appends to the INCLUDE_DIRECTORIES directory property.

Instead, specify these on the target to keep scope as narrow as possible.

…s and include_directories

add_definitions and include_directories don't operate on a by-target basis, they act on a
by-directory basis (i.e. if we defined two targets, A and B, in this CMakeLists file, add_definitions
would add the definitions to the COMPILE_DEFINITIONS directory property which both A and B would
implicitly use).

The same idea applies to include_directories, only it appends to the INCLUDE_DIRECTORIES directory
property.

Instead, specify these on the target to keep scope as narrow as possible.
Copy link
Member

@leoetlino leoetlino left a comment

Choose a reason for hiding this comment

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

LGTM, I was going to make a PR for this when I saw your previous PR :P

@leoetlino leoetlino merged commit a3bdb5d into dolphin-emu:master Apr 5, 2018
@lioncash lioncash deleted the target branch April 5, 2018 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants