-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Switch nav2_smac_planner to modern CMake idioms. #4634
Switch nav2_smac_planner to modern CMake idioms. #4634
Conversation
Signed-off-by: Chris Lalancette <clalancette@gmail.com>
find_package(rcl_interfaces REQUIRED) | ||
find_package(tf2 REQUIRED) | ||
find_package(tf2_ros REQUIRED) | ||
find_package(visualization_msgs REQUIRED) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nav2_package()
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point.
When I turn that on locally, I get a host of new compiler warnings (which are then errors, because of -Werror
). Would you like me to submit a separate PR where we only turn on nav2_package()
and fix the warnings, or do you want me to just fix them all in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A new PR would be fine. Also, if you'd prefer, you could file a ticket with those errors and we can kick that can for another time entirely. I don't think that's critically in your path of what you're aiming to do, so I don't want to pile on - unless you'd like to :-)
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
Signed-off-by: Chris Lalancette <clalancette@gmail.com> Signed-off-by: Joseph Duchesne <josephgeek@gmail.com>
Basic Info
Description of contribution in a few bullet points
Switch nav2_smac_planner to use modern CMake idioms:
Description of documentation updates required from your changes
None needed.
Future work that may be required in bullet points
This is part of a larger series to switch Navigation2 to modern CMake idioms. After this PR, there 3 PRs to go.
For Maintainers: