-
Notifications
You must be signed in to change notification settings - Fork 132
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
Deprecate ament_export_include_directories, _definitions, _libraries, _link_flags #365
Comments
This was referenced Nov 29, 2021
This was referenced Dec 7, 2021
This was referenced Jan 4, 2022
Merged
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
There are a few ament_cmake functions that support old-style standard CMake variables. They are no longer needed in modern CMake. The equivalent standard CMake functions are advantageous to use because they're target based instead of project based. That allows targets in a package to use only the targets they need from another cmake package, potentially saving build time and code size.
Deprecating these methods seems like a good idea because
ament_export_targets()
usage already seems widespread, making the old-style standard variables redundant.ament_export_include_directories()
is a function that makes sure the variablefoobar_INCLUDE_DIRS
gets set when someone callsfind_package(foobar)
. This is an old style "standard CMake variable", and it isn't needed when exporting targets. I think it should be deprecated with a message to usetarget_include_directories()
andament_export_targets()
instead.ament_export_definitions()
causes thefoobar_DEFINITIONS
variable to be set, and can be deprecated in favor oftarget_compile_definitions()
andament_export_targets()
.ament_export_libraries()
causes thefoobar_LIBRARIES
variable to be set, and can be deprecated in favor oftarget_link_libraries()
andament_export_targets()
.ament_export_link_flags()
causes thefoobar_LINK_FLAGS
variable to be set, and can be deprecated in favor oftarget_link_options()
andament_export_targets()
.Somewhat related to #292
The text was updated successfully, but these errors were encountered: