Cleanup CMakeLists.txt - #3824
Conversation
Removed several unnecessary dependencies from target_link_libraries for various executables and libraries in CMakeLists.txt.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Included review availability: Your plan provides up to 3 included reviews per hour; 0 remain after this review. 📝 WalkthroughWalkthroughChangesThe Servo CMake configuration removes redundant link dependencies from Servo libraries, demos, the keyboard executable, and tests. Each target retains only its required direct dependencies. Servo CMake dependency cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: ⚪ Minimal · up to This localized CMake cleanup changes target dependency declarations without any identified merge-blocking risk; it is merge-ready after normal checks and review. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3824 +/- ##
==========================================
- Coverage 48.76% 48.75% -0.00%
==========================================
Files 730 730
Lines 62470 62470
Branches 7609 7609
==========================================
- Hits 30455 30454 -1
- Misses 32015 32016 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
rhaschke
left a comment
There was a problem hiding this comment.
Seems to work. Nice cleanup! Thanks.
|
Tick the box to add this pull request to the merge queue (same as
|
|
@mosfet80: Do you plan to look into other CMakefiles too? |
I also plan to optimize the other packages included in moveit2. Unfortunately, I'm busy with other activities and won't be able to finish it this month. |
|
Great. As this cleanup is not at all urgent, I'm happy to wait until all cleanups are collected here. I'll switch the status to draft. |
christian-rauch
left a comment
There was a problem hiding this comment.
Thanks for the cleanup. I would prefer if you keep the line-by-line target list for easier readability.
| target_link_libraries(moveit_servo_lib_cpp moveit_servo_lib_parameters | ||
| moveit_ros_planning::moveit_ros_planning) |
There was a problem hiding this comment.
Can you keep the CMake targets per line (here and anywhere else)? This makes it easier readable, and you will see in the diff which targets actually get removed.
Description
Removed several unnecessary dependencies from target_link_libraries for various executables and libraries in CMakeLists.txt.
Checklist
Summary by CodeRabbit