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.

Sign up for GitHub

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

Jump to bottom

STYLE: Use ${PROJECT_NAME} as CMake target name of example projects #388

Merged
merged 1 commit into from
May 26, 2022
Merged

STYLE: Use ${PROJECT_NAME} as CMake target name of example projects #388

merged 1 commit into from
May 26, 2022

Conversation

N-Dekker
Copy link
Collaborator

Replaced add_executable\(\S+ with add_executable\(\${PROJECT_NAME},
replaced install\(TARGETS \S+ with install\(TARGETS \${PROJECT_NAME},
and target_link_libraries\(\S+ with target_link_libraries\${PROJECT_NAME},
using Notepad++ v8.3.3, Find in Files (Regular expression, Filters:
CMake*.txt).

This makes the convention of using the project name as target name explicit.

@github-actions github-actions bot added area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Nonunit Issues affecting the Nonunit module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Remotes Issues affecting the Remote module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Style Style changes: no logic impact (indentation, comments, naming) labels May 25, 2022
@N-Dekker N-Dekker marked this pull request as ready for review May 25, 2022 12:47
@N-Dekker N-Dekker marked this pull request as draft May 25, 2022 13:30
@N-Dekker
STYLE: Use ${PROJECT_NAME} as CMake target name of example projects
b018c2b 
Replaced `add_executable\(\S+` with `add_executable\(\${PROJECT_NAME}`,
`target_link_libraries\(\S+` with `target_link_libraries\${PROJECT_NAME}`,
`install\(TARGETS \S+` with `install\(TARGETS \${PROJECT_NAME}`, and
`COMMAND \${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/\S+` with
`COMMAND \${CMAKE_RUNTIME_OUTPUT_DIRECTORY}/\${PROJECT_NAME}`, using
Notepad++ v8.3.3, Find in Files (Regular expression, Filters: `CMake*.txt`).

This makes the convention of using the project name as target name more explicit.
@N-Dekker N-Dekker force-pushed the Use-PROJECT_NAME-in-CMakeLists branch from 8138ab7 to b018c2b Compare May 25, 2022 13:56
@N-Dekker
Copy link
Collaborator Author

FYI: This force-push has adjusted the COMMAND of add_test commands as well.

@N-Dekker N-Dekker marked this pull request as ready for review May 25, 2022 14:14
tbirdso
tbirdso approved these changes May 26, 2022
View reviewed changes
Copy link
Contributor

@tbirdso tbirdso left a comment

Choose a reason for hiding this comment

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

Very nice!

@tbirdso tbirdso merged commit 468ee04 into InsightSoftwareConsortium:master May 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tbirdso tbirdso tbirdso approved these changes

@dzenanz dzenanz dzenanz approved these changes

Assignees
No one assigned
Labels
area:Bridge Issues affecting the Bridge module area:Core Issues affecting the Core module area:Filtering Issues affecting the Filtering module area:IO Issues affecting the IO module area:Nonunit Issues affecting the Nonunit module area:Numerics Issues affecting the Numerics module area:Registration Issues affecting the Registration module area:Remotes Issues affecting the Remote module area:Segmentation Issues affecting the Segmentation module area:Video Issues affecting the Video module type:Infrastructure Infrastructure/ecosystem related changes, such as CMake or buildbots type:Style Style changes: no logic impact (indentation, comments, naming)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@N-Dekker @dzenanz @tbirdso