-
Notifications
You must be signed in to change notification settings - Fork 18
Conversation
This is to allow people who transitively depend on this repo to use stardoc to generate documentation for their Bazel targets.
name = "java_repositories", | ||
srcs = ["java_repositories.bzl"], | ||
visibility = ["//visibility:public"], | ||
# Temporarily omit @bazel_tools//... bzl files from the bzl_library until |
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.
The upstream change lives here: bazelbuild/bazel#12258
@@ -120,3 +107,9 @@ pkg_tar( | |||
strip_prefix = "/", | |||
package_dir = "federation/sample", | |||
) | |||
|
|||
bzl_library( |
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.
I'm not sure why buildifier thinks this should be at the bottom but it is very adamant about it. I can manually bump it back to the top if you'd like.
@@ -1,10 +1,9 @@ | |||
package(default_visibility=["//visibility:public"]) |
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.
Entries for //tests/...
can easily be excluded if you'd prefer they not exist, but they are technically importable by third parties since bzl
files come with an implicit public visibility in the context of Bazel's load
statements unless they live in a directory called private
.
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.
FYI: We are about to rewrite this entire project so it is useful, so this PR might soon be moot. In any case, I think it is a real bug that someone depending on the repo would see the internal code. We need distributions that have only what you need at runtime, the rule version list for a given Bazel version.
I'm not going to get a chance to properly evaluate this until sometime next week.
This is a blocker for documentation generation right now for
Given the example:
I would expect there to be a
I'm not sure what the action is here. |
Sorry. I was just talking about what should/will be rather than what is. I'll shut up because it does not help you get past your immediate problem. Other people are taking over this work, so I don't think it is my choice to merge or not, but from what I see, I think it should be merged. |
Friendly ping on this |
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.
Sorry for the long delay. I don't think I know Federation's setup well enough to review this. I'm indeed working on a new system, but it would likely replace Federation, rather than modify it.
Nevertheless, I'll go ahead and approve this to unblock you (also since @aiuto essentially gave an informal approval above).
This is to allow people who transitively depend on this repo to use
stardoc to generate documentation for their Bazel targets.
For the origin of this commit, please see
bazel-contrib/bazel-gazelle#760 (comment)
and the linked PRs in that thread.