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

Remove ROS from within Drake #6783

Merged
merged 1 commit into from
Aug 15, 2017
Merged

Remove ROS from within Drake #6783

merged 1 commit into from
Aug 15, 2017

Conversation

jamiesnape
Copy link
Contributor

@jamiesnape jamiesnape commented Aug 7, 2017

Closes #3418, closes #3429, closes #4711, closes #5579, and closes #6674.

Relates in part to #5012.


This change is Reviewable

@jamiesnape
Copy link
Contributor Author

+@stonier for feature review.

@stonier
Copy link
Contributor

stonier commented Aug 15, 2017

Need to also remove in the docs:

  • index.rst : and preliminary ROS support
  • code_style_guide.rst : the section on package.xml

If those are done, :lgtm:


Comments from Reviewable

@jwnimmer-tri
Copy link
Collaborator

The style guide section needs to stay in place, at least in part. It reads:

Note that package.xml files are necessary even if you're not using Drake
with ROS because the model files used by Drake (e.g., URDF and SDF), frequently
refer to resources like mesh files via a package:// syntax, whose full paths
are resolved by the package.xml files.

.. and we'll continue to use package.xml to find resources for the foreseeable future, with or without ROS.

@stonier
Copy link
Contributor

stonier commented Aug 15, 2017

Ach, thanks for catching. Awkward setup.

@jamiesnape
Copy link
Contributor Author

and preliminary ROS support has been removed.


Review status: 0 of 35 files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@jamiesnape
Copy link
Contributor Author

+@SeanCurtis-TRI for platform review.

@drake-jenkins-bot
Copy link

Jenkins build passed

@SeanCurtis-TRI
Copy link
Contributor

:LGTM: +@sammy-tri for final platform review


Reviewed 34 of 34 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@sammy-tri
Copy link
Contributor

Didn't actually look at the diffs of the deleted files in ros/ :lgtm: based on the rest


Reviewed 10 of 34 files at r1, 1 of 1 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@sammy-tri sammy-tri merged commit 1e0d670 into RobotLocomotion:master Aug 15, 2017
@jamiesnape jamiesnape deleted the remove-ros branch August 22, 2017 20:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment