-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
[models] Move allegro_hand_description into drake_models #21184
[models] Move allegro_hand_description into drake_models #21184
Conversation
4096e3d
to
0d11bc2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+@rpoyner-tri for both reviews, please +(status: single reviewer ok).
Reviewable status: 1 unresolved discussion, LGTM missing from assignee rpoyner-tri(platform)
examples/allegro_hand/test/parse_test.cc
line 22 at r2 (raw file):
TEST_P(ParseTest, Quantities) { const std::string file_extension = GetParam(); const std::string url_right =
FYI This test file is moved and edited, not created from scratch, but sadly git isn't noticing the rename.
The only change to the test was fixing these package URLs.
tools/workspace/drake_models/repository.bzl
line 9 at r1 (raw file):
name = name, repository = "RobotLocomotion/models", commit = "bc0447998cbba9046323581775d4ea7f5d00a1a2",
Working
Replace with the real commit sha from master, once we have it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 2 unresolved discussions, LGTM missing from assignee rpoyner-tri(platform)
BUILD.bazel
line 43 at r2 (raw file):
"//bindings/pydrake/multibody:models", "//examples/acrobot:models", "//examples/allegro_hand:models",
nit Actually this line doesn't accomplish anything. I'll remove it in the next push.
0d11bc2
to
5a5523b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 12 of 14 files at r1, 2 of 2 files at r2, 2 of 2 files at r3, all commit messages.
Reviewable status: 2 unresolved discussions
tools/workspace/drake_models/test/parse_test.py
line 65 at r3 (raw file):
"package://drake_models/atlas/robotiq.urdf", "package://drake_models/atlas/robotiq_simple.urdf", # We don't any tracking issue for fixing the allegro models,
typo
Suggestion:
don't have any
5a5523b
to
754a78a
Compare
754a78a
to
874e328
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 2 of 2 files at r4, all commit messages.
Reviewable status: complete! all discussions resolved, LGTM from assignee rpoyner-tri(platform)
Towards #13942.
Twin PR: RobotLocomotion/models#40
This change is