[doc,tools] Adjust imports and libraries in prep for bzlmod #21453
Conversation
jwnimmer-tri
added
priority: low
status: single reviewer ok
|
+@SeanCurtis-TRI for both reviews, please (load balancing). |
|
@drake-jenkins-bot mac-arm-ventura-clang-bazel-experimental-release please. |
There was a problem hiding this comment.
Reviewed 41 of 41 files at r1, all commit messages.
Reviewable status: 3 unresolved discussions
a discussion (no related file):
tools/install/dummy/BUILD.bazel still uses py_library over drake_py_library. As the name includes "dummy" I can certainly entertain arguments for it not mattering. But, I'd opt for some explicitly handling (like in tools/jupyter/BUILD.bazel).
a discussion (no related file):
tools/lint/python_lint.bzl has some example code in the documentation referencing py_library instead of drake_py_library. Do we want to update the example?
tools/install/BUILD.bazel line 8 at r1 (raw file):
"drake_py_unittest", ) load("//tools/skylark:py.bzl", "py_library")
nit unused
|
@drake-jenkins-bot mac-arm-ventura-clang-bazel-experimental-release please. |
Re-spell imports to lose the "drake" part of the module namespace. The old spelling of imports is dispreferred and will not work with upcoming bzlmod. For all Drake Python code, we should be using drake_py_library so that all Drake-specific settings are always in effect. (Earlier in bzlmod porting this was critically important; since then, I've changed some options so that it's less critical but we should anyway follow this hygiene so that we always have a single point of control for our build rules.)
jwnimmer-tri
force-pushed
the
bzlmod-doctools-import-path
branch
from
3aa59f9 to
1426f9c
Compare
There was a problem hiding this comment.
Reviewable status: 2 unresolved discussions
a discussion (no related file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
tools/lint/python_lint.bzlhas some example code in the documentation referencingpy_libraryinstead ofdrake_py_library. Do we want to update the example?
No, it's supposed to be docs available for downstream use also. But anyway I've been slowing purging the "Example" snippets from those docs (because they bitrot) so purged now.
a discussion (no related file):
Previously, SeanCurtis-TRI (Sean Curtis) wrote…
tools/install/dummy/BUILD.bazelstill usespy_libraryoverdrake_py_library. As the name includes "dummy" I can certainly entertain arguments for it not mattering. But, I'd opt for some explicitly handling (like intools/jupyter/BUILD.bazel).
Aha. Good reminder.
When I visited that file while developing this PR, I had thought it was a sample directory for a downstream project, in which case using "drake" macros would be incorrect.
Looking again now, I see it's supposed to be a "pretend to be a really stubby form of Drake itself" project, so we should be using the "drake" macros.
There was a problem hiding this comment.
Reviewed 4 of 4 files at r2, all commit messages.
Reviewable status:complete! all discussions resolved, LGTM from assignee SeanCurtis-TRI(platform)
…omotion#21453) Re-spell imports to lose the "drake" part of the module namespace. The old spelling of imports is dispreferred and will not work with upcoming bzlmod. For all Drake Python code, we should be using drake_py_library so that all Drake-specific settings are always in effect. (Earlier in bzlmod porting this was critically important; since then, I've changed some options so that it's less critical but we should anyway follow this hygiene so that we always have a single point of control for our build rules.)
Re-spell imports to lose the "drake" part of the module namespace. The old spelling of imports is dispreferred and will not work with upcoming bzlmod.
For all Drake Python code, we should be using drake_py_library so that all Drake-specific settings are always in effect. (Earlier in bzlmod porting this was critically important; since then, I've changed some options so that it's less critical but we should anyway follow this hygiene so that we always have a single point of control for our build rules.)
Towards #20731. See #21401 for prior art.
This change is