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.

Sign up for GitHub

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

Jump to bottom

have moveit_planners depend on chomp #3015

Merged
merged 2 commits into from
Oct 6, 2024
Merged

have moveit_planners depend on chomp #3015

merged 2 commits into from
Oct 6, 2024

Conversation

mikeferguson
Copy link
Contributor

Description

By default, the packages generated by moveit_setup_assistant now include a chomp configuration - but do not depend on chomp. the packages do depend on moveit_planners, so it makes sense to include chomp here.

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

@mikeferguson
have moveit_planners depend on chomp
16371eb 
by default, the packages generated by moveit_setup_assistant now
include a chomp configuration - but do not depend on chomp. the
packages do depend on moveit_planners, so it makes sense to include
chomp here.
sjahr
sjahr approved these changes Sep 30, 2024
View reviewed changes
Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

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

Thanks for the quick fix! Not blocking this PR but would you mind to delete the other commented out moveit_planners_chomp exec_depend. I think it doesn't exist anyways

@mikeferguson
Copy link
Contributor Author

Thanks for the quick fix! Not blocking this PR but would you mind to delete the other commented out moveit_planners_chomp exec_depend. I think it doesn't exist anyways

Updated - hadn't noticed that it was also chomp - I only looked for the one package that I was trying to include!

@sjahr sjahr merged commit b88af32 into moveit:main Oct 6, 2024
8 of 13 checks passed
@mikeferguson mikeferguson deleted the depend_chomp branch October 8, 2024 20:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sjahr sjahr sjahr approved these changes

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
@mikeferguson @sjahr