-
Couldn't load subscription status.
- Fork 1.3k
[doc] Add instructions to upgrade DEE CMake externals #23635
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
[doc] Add instructions to upgrade DEE CMake externals #23635
Conversation
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.
-(status: do not merge) -(status: do not review) +a:@Aiden2244 for feature review, please.
Reviewable status: LGTM missing from assignee Aiden2244, needs platform reviewer assigned, needs at least two assigned reviewers, labeled "do not merge" (waiting on @tyler-yankee)
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, needs platform reviewer assigned, needs at least two assigned reviewers
-- commits line 2 at r1:
nit "DEE packages" is a little vague to me, I might suggest "...upgrade DEE CMake externals" to be a little more specific and match the naming used in the new python script.
tools/workspace/README.md line 198 at r1 (raw file):
process. The packages are listed in Drake's `MODULE.bazel`, and the corresponding user-
nit at the risk of being overly pedantic, "user-provided" should be newlined as a single word to avoid an erroneous space after the hyphen when this text is rendered on GitHub.
5648cd6 to
94f91ba
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.
thanks!
Possibly +a:@jwnimmer-tri would have more context on this PR for platform than regular on-call? Feel free to delegate as needed.
Reviewable status: LGTM missing from assignee jwnimmer-tri(platform)
Previously, Aiden2244 (Aiden McCormack) wrote…
nit "DEE packages" is a little vague to me, I might suggest "...upgrade DEE CMake externals" to be a little more specific and match the naming used in the new python script.
done
tools/workspace/README.md line 198 at r1 (raw file):
Previously, Aiden2244 (Aiden McCormack) wrote…
nit at the risk of being overly pedantic, "user-provided" should be newlined as a single word to avoid an erroneous space after the hyphen when this text is rendered on GitHub.
done
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.
@jwnimmer-tri reviewed 1 of 1 files at r2, all commit messages.
Reviewable status: 1 unresolved discussion
tools/workspace/README.md line 198 at r2 (raw file):
process. The packages are listed in Drake's `MODULE.bazel`, and the corresponding
BTW This sentence could probably be more precise that thedrake/MODULE.bazel are the canonical versions (and drake_cmake_external/CMakeLists.txt are the derivative versions that should follow along).
94f91ba to
17427c9
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.
Reviewable status: 1 unresolved discussion
tools/workspace/README.md line 198 at r2 (raw file):
Previously, jwnimmer-tri (Jeremy Nimmer) wrote…
BTW This sentence could probably be more precise that the
drake/MODULE.bazelare the canonical versions (anddrake_cmake_external/CMakeLists.txtare the derivative versions that should follow along).
I think I simplified while adding the clarification you were looking for, lmk your thoughts.
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.
@jwnimmer-tri reviewed 1 of 1 files at r3, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignees jwnimmer-tri(platform),Aiden2244
tools/workspace/README.md line 198 at r2 (raw file):
Previously, tyler-yankee (Tyler Yankee) wrote…
I think I simplified while adding the clarification you were looking for, lmk your thoughts.
Definitely better.
Towards RobotLocomotion/drake-external-examples#432 (see also: RobotLocomotion/drake-external-examples#459).
This change is