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

Decorate MoveItConfigs with dataclass #1308

Merged
merged 1 commit into from
Jun 3, 2022

Conversation

JafarAbdi
Copy link
Contributor

Description

Fix: #1292 (comment), this reduces the duplication and make the class easier to read

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 Jun 2, 2022

Codecov Report

Merging #1308 (43a3c3f) into main (cea5764) will decrease coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1308      +/-   ##
==========================================
- Coverage   61.48%   61.46%   -0.01%     
==========================================
  Files         274      274              
  Lines       24940    24940              
==========================================
- Hits        15332    15328       -4     
- Misses       9608     9612       +4     
Impacted Files Coverage Δ
...nning_scene_monitor/src/planning_scene_monitor.cpp 45.29% <0.00%> (-0.43%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cea5764...43a3c3f. Read the comment docs.

Copy link
Contributor

@vatanaksoytezer vatanaksoytezer left a comment

Choose a reason for hiding this comment

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

Looks much cleaner!

@henningkayser henningkayser changed the title Decorate MoveItConfigs with dataclaass Decorate MoveItConfigs with dataclass Jun 2, 2022
@JafarAbdi JafarAbdi force-pushed the pr-moveit_configs_dataclass branch from 8c88efa to 28f67ec Compare June 2, 2022 17:37
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.

This looks very nice! I assume this is fully compatible with the old version?

@JafarAbdi
Copy link
Contributor Author

This looks very nice! I assume this is fully compatible with the old version?

This uses a feature from Python 3.10, so it only work with Jammy

@JafarAbdi JafarAbdi force-pushed the pr-moveit_configs_dataclass branch from 28f67ec to 43a3c3f Compare June 2, 2022 18:18
@henningkayser henningkayser added the backport-humble Mergify label that triggers a PR backport to Humble label Jun 3, 2022
@henningkayser henningkayser merged commit 4502659 into moveit:main Jun 3, 2022
mergify bot pushed a commit that referenced this pull request Jun 3, 2022
henningkayser pushed a commit that referenced this pull request Jun 6, 2022
(cherry picked from commit 4502659)

Co-authored-by: Jafar <cafer.abdi@gmail.com>
peterdavidfagan pushed a commit to peterdavidfagan/moveit2 that referenced this pull request Jul 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble
Projects
None yet
Development

Successfully merging this pull request may close these issues.

WMD Track: MoveIt Config Utils
3 participants