Skip to content
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

Setting visibility of image_bzl to public so that it can be used when… #1917

Merged
merged 1 commit into from
Oct 7, 2021

Conversation

ngeor
Copy link
Contributor

@ngeor ngeor commented Jul 23, 2021

… documenting custom rules with stardoc (Fixes #1916)

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

  • [ x] Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • [ x] No

Other information

@google-cla
Copy link

google-cla bot commented Jul 23, 2021

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Jul 23, 2021
@ngeor
Copy link
Contributor Author

ngeor commented Jul 23, 2021

@googlebot I signed it!

@google-cla google-cla bot added cla: yes and removed cla: no labels Jul 23, 2021
@ngeor
Copy link
Contributor Author

ngeor commented Aug 10, 2021

Good morning folks, can I please get a review? 🙏 @alexeagle @gravypod @pcj @smukherj1

@alexeagle
Copy link
Collaborator

I'm surprised if the issue is only in Java - should we change this everywhere?
Note that you can just pass --nocheck_visibility to Bazel to bypass the checking when you build your stardoc target

@ngeor
Copy link
Contributor Author

ngeor commented Aug 11, 2021

Right, I only use Java so that's why I reported it only for Java :) From some colleague who is more experienced with Bazel, he mentioned rules_docker might be a bit inconsistent regarding visibility. Let me see if I can use the flag you mentioned.

@ngeor
Copy link
Contributor Author

ngeor commented Aug 11, 2021

I gave it a try, with a mix of sometimes referencing bzl files and sometimes bazel_library targets, I got it to work. But, I can't actually use the --nocheck_visibility flag because that means every single build command we run against our repo (e.g. bazel build //...) won't work anymore unless we specify that flag.

@ngeor
Copy link
Contributor Author

ngeor commented Aug 11, 2021

Ok, my colleague advised me to use patches. I am applying patches on top of rules_docker. I should've done this before I created the MR, apologies for this. This revealed two more issues:

  • another file we need to be changed is repositories/BUILD. As you predicted, more files might be needed to be public for other people, but for me just these two suffice. I opted in my patch to just make a library with a wildcard:

bzl_library(
    name = "repositories",
    srcs = glob(["*.bzl"]),
)

Furthermore, I see I need to patch another repo, Bazel Gazelle, because you use it and they don't even provide a bazel_library. I patched that as well and then I was able to use stardoc in a more sane way.

… documenting custom rules with stardoc (Fixes #1916)
@ngeor
Copy link
Contributor Author

ngeor commented Aug 12, 2021

Related PR for gazelle bazel-contrib/bazel-gazelle#1092

Copy link
Collaborator

@alexeagle alexeagle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for fixing in gazelle too :)

@alexeagle alexeagle merged commit 9252c6a into bazelbuild:master Oct 7, 2021
@ngeor
Copy link
Contributor Author

ngeor commented Oct 8, 2021

Thank you for merging @alexeagle , I took the latest git sha from rules_docker and gazelle and I was able to use stardoc to auto-generate the documentation for our Bazel rules/macros!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't use stardoc to document custom rules that depend on java_image
2 participants