Skip to content

Port to ROS 2 dashing (new branch)#118

Closed
henningkayser wants to merge 4 commits intomoveit:dashing-develfrom
PickNikRobotics:dashing-devel
Closed

Port to ROS 2 dashing (new branch)#118
henningkayser wants to merge 4 commits intomoveit:dashing-develfrom
PickNikRobotics:dashing-devel

Conversation

@henningkayser
Copy link
Copy Markdown
Member

This is a fixup of the previous ROS 2 migration attempts and synced with melodic-devel.
I created the 'new' dashing-devel based on melodic devel, so this PR only lists the migration steps.
When this gets merged, I suggest we remove crystal-devel and close the PRs #96 and #110

@henningkayser henningkayser changed the title New ROS 2 dashing Port to ROS 2 dashing (new branch) Nov 15, 2019
Copy link
Copy Markdown
Contributor

@rhaschke rhaschke left a comment

Choose a reason for hiding this comment

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

Hi Henning, it would be great if you could cleanup the commit history somewhat and have individual commits for:

  • msg related changes
  • cmake/catkin/ament related changes
  • real code changes (probably none)

@rhaschke
Copy link
Copy Markdown
Contributor

When this gets merged, I suggest we remove crystal-devel and close the PRs #96 and #110.

Fully agree.

@henningkayser henningkayser force-pushed the dashing-devel branch 2 times, most recently from 6993192 to db38028 Compare November 15, 2019 12:38
@henningkayser henningkayser force-pushed the dashing-devel branch 5 times, most recently from f5eb936 to eae9fb0 Compare November 15, 2019 14:47
@henningkayser
Copy link
Copy Markdown
Member Author

henningkayser commented Nov 15, 2019

Hi Henning, it would be great if you could cleanup the commit history somewhat and have individual commits for:

  • msg related changes
  • cmake/catkin/ament related changes
  • real code changes (probably none)

@rhaschke I squashed all commits into a more readable history and applied your requested changes. Travis fails because of warnings in octomap and because there is no CMAKE_BUILD_TYPE defined. For the latter we need to pass the parameter to colcon build which requires changes in moveit_ci. Since there is nothing to add for this PR, lgtm.

This was referenced Nov 15, 2019
@rhaschke
Copy link
Copy Markdown
Contributor

Rebased onto latest HEAD in #122.

@rhaschke rhaschke closed this Nov 17, 2019
This was referenced Nov 17, 2019
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.

3 participants