Skip to content

Commit

Permalink
cmake: target_compile_features instead of set_property
Browse files Browse the repository at this point in the history
  • Loading branch information
@zdenop
zdenop committed Mar 30, 2024
1 parent 2b07505 commit 87a152c
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion src/training/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ if(ICU_FOUND)
# ############################################################################

add_executable(unicharset_extractor unicharset_extractor.cpp)
set_property(TARGET unicharset_extractor PROPERTY CXX_STANDARD 17)
target_compile_features(unicharset_extractor PRIVATE cxx_std_17)
target_link_libraries(unicharset_extractor unicharset_training)
project_group(unicharset_extractor "Training Tools")
install(
Expand Down

6 comments on commit 87a152c

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 87a152c Mar 30, 2024

Choose a reason for hiding this comment

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

Zdenko,

Maybe we should raise the minimun required version of CMake.

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 87a152c Mar 30, 2024

Choose a reason for hiding this comment

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

To version 3.16, which is supported by Ubuntu 20.04, Debian 10 and RHEL 8.

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 87a152c Mar 30, 2024 •
edited

Choose a reason for hiding this comment

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

And the latest version of SLES/openSUSE.

@zdenop
Copy link
Contributor Author

@zdenop zdenop commented on 87a152c Mar 30, 2024

Choose a reason for hiding this comment

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

Thanks for checking.
I am a little bit hesitant about raising the minimum version limit as I am not aware of the function that needs it (in this case target_compile_features is supported from 3.1).

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 87a152c Mar 31, 2024

Choose a reason for hiding this comment

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

This is was an off-topic comment.

It's a general comment about CMake, not specific to your patch.

Just a suggestion you can consider, if you find something you can use that is only available in a newer version than the current minimum version.

@amitdo
Copy link
Collaborator

@amitdo amitdo commented on 87a152c Mar 31, 2024

Choose a reason for hiding this comment

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

But if you want to keep the old minimum for some more years, it's fine.

Please sign in to comment.