-
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
Add drake_cc_package_library and library_lint #8582
Add drake_cc_package_library and library_lint #8582
Conversation
f32bbe9
to
ffaf022
Compare
226dafa
to
491bba9
Compare
+@jamiesnape for design review (or as much code review as you wish also), please. |
491bba9
to
c30a329
Compare
Reviewed 41 of 41 files at r1. tools/skylark/drake_cc.bzl, line 346 at r1 (raw file):
BTW Spelling tools/skylark/drake_cc.bzl, line 360 at r1 (raw file):
BTW Grammar "convenient" should be "conveniently" Comments from Reviewable |
c30a329
to
a767d4e
Compare
@jamiesnape any thoughts? +@sammy-tri for platform review per schedule (tomorrow), please. Review status: 39 of 41 files reviewed at latest revision, all discussions resolved. Comments from Reviewable |
modulo minor typo issue. I can't say I love the Reviewed 38 of 41 files at r1, 2 of 2 files at r2. automotive/maliput/api/BUILD.bazel, line 4 at r2 (raw file):
BTW Why do we respell this tools/lint/library_lint.bzl, line 15 at r2 (raw file):
typo "then" instead of "they"? Comments from Reviewable |
a767d4e
to
c7fe7c1
Compare
Review status: 40 of 41 files reviewed at latest revision, 2 unresolved discussions. a discussion (no related file):
FYI @sammy-tri In an earlier local draft of this change, I actually had a way to bless the existing cc_library into a cc_package_library without creating an automotive/maliput/api/BUILD.bazel, line 4 at r2 (raw file): Previously, sammy-tri (Sam Creasey) wrote…
The I don't change tools/lint/library_lint.bzl, line 15 at r2 (raw file): Previously, sammy-tri (Sam Creasey) wrote…
Done. Comments from Reviewable |
Reviewed 1 of 41 files at r1, 1 of 1 files at r3. Comments from Reviewable |
Review status: all files reviewed at latest revision, all discussions resolved, some commit checks failed. Comments from Reviewable |
c7fe7c1
to
ab9732b
Compare
This should be substantially easier to maintain correct lists, and document for authors the purpose of the package-level library without repeating identical comments everywhere.
This is also a step along the way to #6464, where libdrake will depend on just these package-summary libraries, instead of listing out each individual component.
This change is