-
Notifications
You must be signed in to change notification settings - Fork 180
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
Gazelle now handles imports from @bazel_tools
#273
Conversation
9b918cb
to
fefba30
Compare
@segiddins I believe this addresses one of your manual changes. Could I have you give it a once over to verify it does. @jayconrod could I have you take a look at this? Thanks so much! |
6b5f40d
to
b1f7d55
Compare
I've figured out how to satisfy buildifier's vengeful constraints and all the tests are green now and ready for review. Sorry about that! |
@achew22 I was adding the |
LGTM, but I don't have mergeability. |
b1f7d55
to
5c21350
Compare
@segiddins PTAL |
@jayconrod could I have you poke this one more time. I think (hope?) that your approval is now sufficient. |
`@bazel_tools` is tricky since it is effectively a part of the standard library that can not have a `bzl_library` attached to it. As a simple fix for this, `bzl_library` can have a srcs dependency on it so that it includes the transitive closure of all of its dependencies. `@bazel_tools` imports are rewritten into the `srcs` attribute since they are `exports_files`ed from the @bazel_tools.
5c21350
to
a71dc18
Compare
Approved, but I still can't merge anything in this repo. |
|
@c-parsons friendly ping on this |
Hmm, I'm not sure why Jay's approval still isn't working. :( |
@bazel_tools
is tricky since it is effectively a part of the standardlibrary that can not have a
bzl_library
attached to it. As a simplefix for this,
bzl_library
can have a srcs dependency on it so that itincludes the transitive closure of all of its dependencies.
@bazel_tools
imports are rewritten into thesrcs
attribute sincethey are
exports_files
ed from the @bazel_tools.