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
Replaced boost::algorithm::join with fmt::join #2273
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #2273 +/- ##
==========================================
- Coverage 50.72% 50.68% -0.03%
==========================================
Files 386 386
Lines 31914 31914
==========================================
- Hits 16184 16173 -11
- Misses 15730 15741 +11
☔ View full report in Codecov by Sentry. |
@sjahr |
Thank you! This CI failure is not caused by your changes, you can ignore it 👍 |
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.
Looks good, thank you! I still have to test it. Can we remove the Boost dependency in some CMakeLists.txt
or package.xml
files?
moveit_ros/move_group/CMakeLists.txt
Outdated
@@ -65,7 +66,7 @@ target_link_libraries(move_group moveit_move_group_capabilities_base) | |||
|
|||
add_executable(list_move_group_capabilities src/list_capabilities.cpp) | |||
ament_target_dependencies(list_move_group_capabilities ${THIS_PACKAGE_INCLUDE_DEPENDS} Boost) |
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.
Is the Boost dependency still 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.
Not for this particular node, I have removed it in the latest commit.
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.
LGTM!
Would be nice to also be able to remove the boost::algorithm::trim()
calls in planning_scene.cpp
, but there doesn't seem to be an easy alternative.
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.
LGTM
This pull request is in conflict. Could you fix it @Shobuj-Paul? |
Description
This PR replaces
boost::algorithm::join
withfmt::join
for string formatting to reduce boost dependencies.Fixes #2227
Checklist