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

Migrate LaserScanDisplay #238

Merged
merged 11 commits into from Apr 2, 2018

Conversation

greimela-si
Copy link
Contributor

Now that ros-perception/laser_geometry#28 has been merged we can migrate the LaserScanDispay to RViz2.

Requirement: ros/angles:ros2 and ros-perception/laser_geometry:ros2 must be added to ros2.repos before merging.

Resolves #79.

@greimela-si
Copy link
Contributor Author

CI including the two additional repositories:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

@wjwwood wjwwood added the in progress Actively being worked on (Kanban column) label Mar 31, 2018
@wjwwood
Copy link
Member

wjwwood commented Mar 31, 2018

I resolved the conflicts in this pr and made a pr for adding the new repositories: ros2/ros2#477

Here's a new CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

two nitpicks, which I might not get to this afternoon, but otherwise lgtm

@@ -117,9 +120,11 @@ target_link_libraries(rviz_default_plugins
rviz_rendering::rviz_rendering
rviz_common::rviz_common
resource_retriever::resource_retriever
laser_geometry::laser_geometry
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: alphabetical order unless the library order matters (I don't think it does)

@@ -32,6 +32,7 @@
<depend>rviz_rendering</depend>
<depend>urdf</depend>
<depend>visualization_msgs</depend>
<depend>laser_geometry</depend>
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: alphabetical order

@wjwwood wjwwood added in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 31, 2018
@wjwwood wjwwood merged commit b19a1ab into ros2:ros2 Apr 2, 2018
@wjwwood wjwwood removed the in review Waiting for review (Kanban column) label Apr 2, 2018
@greimela-si greimela-si deleted the feature/migrate_laser_scan_display branch April 5, 2018 08:44
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

4 participants