Skip to content

Add documentation and cleanups for PlanningRequestAdapter and PlanningRequestAdapterChain classes#2142

Merged
sjahr merged 4 commits intomoveit:mainfrom
sjahr:pr-add_documentation_for_plan_request_adapters
May 12, 2023
Merged

Add documentation and cleanups for PlanningRequestAdapter and PlanningRequestAdapterChain classes#2142
sjahr merged 4 commits intomoveit:mainfrom
sjahr:pr-add_documentation_for_plan_request_adapters

Conversation

@sjahr
Copy link
Copy Markdown
Contributor

@sjahr sjahr commented Apr 26, 2023 •
edited
Loading

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
Copy Markdown

codecov bot commented Apr 26, 2023 •
edited
Loading

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
Open
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
henningkayser
henningkayser reviewed May 8, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@henningkayser henningkayser henningkayser approved these changes

@tylerjw tylerjw Awaiting requested review from tylerjw

@AndyZe AndyZe Awaiting requested review from AndyZe

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@sjahr @henningkayser