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

Move meshes and models from drake to drake_models #23

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

jwnimmer-tri
Copy link
Contributor

@jwnimmer-tri jwnimmer-tri commented Apr 7, 2023

Towards RobotLocomotion/drake#19078 and RobotLocomotion/drake#11913.

See RobotLocomotion/drake#19160 for the sibling pull request.


This change is Reviewable

Copied from Drake git sha 3f9800b1ebcd451e6042b3c5b302ebe3ba182061.
@jwnimmer-tri
Copy link
Contributor Author

+@rpoyner-tri for feature review, please to match up with RobotLocomotion/drake#19160.

Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

:lgtm: conversion scripts question

I verified that (almost) all of the files that came from drake were copied without changes, and the few that were changed only had internal file paths updated.

Reviewed 316 of 316 files at r1, all commit messages.
Reviewable status: 1 unresolved discussion, platform LGTM from [rpoyner-tri] (waiting on @jwnimmer-tri)

a discussion (no related file):
Do the conversion script files need more documentation? I assume they are not needed to be used, but rather are a record of the derivation of some of the files.


Copy link
Contributor Author

@jwnimmer-tri jwnimmer-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: 1 unresolved discussion, platform LGTM from [rpoyner-tri] (waiting on @rpoyner-tri)

a discussion (no related file):

Previously, rpoyner-tri (Rick Poyner (rico)) wrote…

Do the conversion script files need more documentation? I assume they are not needed to be used, but rather are a record of the derivation of some of the files.

I think you're talking about some of the Atlas files. Is that right?

I think only moved those without any edits. If I made them worse (e.g., filenames are wrong now) then let me know and I should fix that here.

If your observation is that some old pre-TRI stuff from 2014 no longer meets our standards, I would agree but I don't think that's relevant to this PR.


Copy link
Contributor

@rpoyner-tri rpoyner-tri left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all discussions resolved, platform LGTM from [rpoyner-tri] (waiting on @jwnimmer-tri)

a discussion (no related file):

Previously, jwnimmer-tri (Jeremy Nimmer) wrote…

I think you're talking about some of the Atlas files. Is that right?

I think only moved those without any edits. If I made them worse (e.g., filenames are wrong now) then let me know and I should fix that here.

If your observation is that some old pre-TRI stuff from 2014 no longer meets our standards, I would agree but I don't think that's relevant to this PR.

fair.


@rpoyner-tri rpoyner-tri merged commit 611246c into RobotLocomotion:master Apr 11, 2023
@jwnimmer-tri jwnimmer-tri deleted the meshathon branch April 11, 2023 21:35
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.

2 participants