-
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
Remove (most) examples from CMake build #6441
Remove (most) examples from CMake build #6441
Conversation
+@jwnimmer-tri for feature review. |
Reviewed 34 of 34 files at r1. a discussion (no related file): a discussion (no related file): drake/examples/CMakeLists.txt, line 2 at r1 (raw file):
This file is dead code and is not known to Bazel. Can we either remove it in this PR, or elsewhere track that we need to remove it? drake/examples/kuka_iiwa_arm/CMakeLists.txt, line 104 at r1 (raw file):
I'm not sure why only this one thing gets removed? Is this per some kind of separate discussion with the kuka team? Comments from Reviewable |
Review status: all files reviewed at latest revision, 4 unresolved discussions. drake/examples/CMakeLists.txt, line 2 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Sure. drake/examples/kuka_iiwa_arm/CMakeLists.txt, line 104 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Actually, all the subdirectories should go. Will fix. Comments from Reviewable |
drake/examples/kuka_iiwa_arm/CMakeLists.txt, line 104 at r1 (raw file): Previously, jamiesnape (Jamie Snape) wrote…
Still unclear on why only some of the CMake goes and some stays (i.e., what's the dividing line). Hold-over glue for Spartan transition? Comments from Reviewable |
Review status: all files reviewed at latest revision, 4 unresolved discussions. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Sure, unless the plan has changed from the days of CMake, @RussTedrake wanted all the examples to be part of redistributable packages.. Comments from Reviewable |
Review status: all files reviewed at latest revision, 3 unresolved discussions. a discussion (no related file): Previously, jamiesnape (Jamie Snape) wrote…
Should we block CMakeLists turn-down on verifying that an appropriate a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
All good; Comments from Reviewable |
Reviewed 6 of 7 files at r2. Comments from Reviewable |
Review status: 39 of 40 files reviewed at latest revision, 2 unresolved discussions. a discussion (no related file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
We could, though in practice, CMake did not package them either. I can add something to this PR. drake/examples/kuka_iiwa_arm/CMakeLists.txt, line 104 at r1 (raw file): Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
Exactly: https://github.com/RobotLocomotion/spartan/blob/master/cmake/modules/FindDrake.cmake#L29 Comments from Reviewable |
Reviewed 2 of 7 files at r2. a discussion (no related file): Previously, jamiesnape (Jamie Snape) wrote…
OK I'm satisfied either way here, as long as there is some rationale for where we cross-check that everything is installed -- either incrementally in the CMake turn-down PRs, or in an "install all the things" PR based on drake/examples/kuka_iiwa_arm/CMakeLists.txt, line 104 at r1 (raw file): Previously, jamiesnape (Jamie Snape) wrote…
OK I'm convinced. Can you be sure to ping slack Comments from Reviewable |
+@ggould-tri for platform review. |
Reviewed 33 of 34 files at r1, 7 of 7 files at r2. Comments from Reviewable |
Toward #3129. Leaving Atlas (used by drake/systems) and kuka_iiwa_arm (used by spartan).
This change is