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

Add documentation and cleanups for PlanningRequestAdapter and PlanningRequestAdapterChain classes #2142

Merged

Conversation

sjahr
Copy link
Contributor

@sjahr sjahr commented Apr 26, 2023

Description

  • Documentation & Cleanups for PlanningRequestAdapter & PlanningRequestAdapterChain classes

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@codecov
Copy link

codecov bot commented Apr 26, 2023

Codecov Report

Patch coverage: 14.29% and project coverage change: +0.01 🎉

Comparison is base (f807733) 50.47% compared to head (58e449f) 50.48%.

❗ Current head 58e449f differs from pull request most recent head 14fa91e. Consider uploading reports for the commit 14fa91e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2142      +/-   ##
==========================================
+ Coverage   50.47%   50.48%   +0.01%     
==========================================
  Files         387      387              
  Lines       31790    31785       -5     
==========================================
  Hits        16043    16043              
+ Misses      15747    15742       -5     
Impacted Files Coverage Δ
...core/planning_interface/src/planning_interface.cpp 60.00% <0.00%> (ø)
...or/src/current_state_monitor_middleware_handle.cpp 88.89% <0.00%> (ø)
...include/moveit_setup_app_plugins/launch_bundle.hpp 0.00% <0.00%> (ø)
...ework/include/moveit_setup_framework/templates.hpp 25.00% <ø> (ø)
...sistant/moveit_setup_framework/src/srdf_config.cpp 68.58% <0.00%> (ø)
.../moveit_setup_srdf_plugins/src/planning_groups.cpp 8.20% <0.00%> (ø)
...assistant/moveit_setup_framework/src/templates.cpp 67.86% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sjahr sjahr mentioned this pull request May 2, 2023
6 tasks
@sjahr sjahr force-pushed the pr-add_documentation_for_plan_request_adapters branch from 13f5d1f to 12073ce Compare May 2, 2023 16:03
Copy link
Member

@henningkayser henningkayser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve these changes in general. But why did you mix in the fixes for MSA and PSM, they don't seem related at all?

@sjahr
Copy link
Contributor Author

sjahr commented May 8, 2023

I approve these changes in general. But why did you mix in the fixes for MSA and PSM, they don't seem related at all?

I thought it makes sense to apply the return std::string(); mini fix everywhere (probably should have done this in a separate PR, sorry) and the variables_ renaming was due to the CI error.

@sjahr sjahr force-pushed the pr-add_documentation_for_plan_request_adapters branch from 58e449f to 1d9778e Compare May 11, 2023 10:02
@sjahr
Copy link
Contributor Author

sjahr commented May 11, 2023

@henningkayser I cleaned it up

@sjahr sjahr requested a review from henningkayser May 11, 2023 10:03
@sjahr sjahr enabled auto-merge (squash) May 12, 2023 07:13
@sjahr sjahr merged commit 226f81c into moveit:main May 12, 2023
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants