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

Rename carma_package(...) and carma_check_ros_version(...) #217

Open
adamlm opened this issue Aug 31, 2023 · 0 comments
Open

Rename carma_package(...) and carma_check_ros_version(...) #217

adamlm opened this issue Aug 31, 2023 · 0 comments
Labels
enhancement New feature or request

Comments

@adamlm
Copy link
Contributor

adamlm commented Aug 31, 2023

Summary

The carma_package(...) and carma_check_ros_version(...) CMake macros are a bit vague and don't clearly represent what they do.

  • carma_package(...): I assume this command was named after the ament_package(...) function, but the two do different tasks. ament_package(...) adds CMake install targets and other packaging configurations for the ROS 2 package. carma_pacakge(...), however, sets the C and C++ standards and add some compiler flags. The ament_package(...) function does what it's name implies---packages up the code for installation---but carma_package(...) does not. I suggest we rename this macro to carma_apply_common_package_configuration(...) or something similar.

  • carma_check_ros_version(...): This macro name is more indicative of its purpose than carma_package(...), but it is still unclear what happens after the check. Does some flag get raised? Does a warning get issued? I suggest renaming this macro to carma_assert_ros_version(...) or something similar, which would better indicate that the CMake configuration stops here if the assertion fails. This is similar to assert and static_asssert in C and C++.

Reasoning for new functionality

See above.

@adamlm adamlm added the enhancement New feature or request label Aug 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant