Skip to content

requirements.bzl is now visible outside pip_repositorys#463

Merged
alexeagle merged 2 commits intobazel-contrib:masterfrom
UebelAndre:docs
Apr 21, 2021
Merged

requirements.bzl is now visible outside pip_repositorys#463
alexeagle merged 2 commits intobazel-contrib:masterfrom
UebelAndre:docs

Conversation

@UebelAndre
Copy link
Copy Markdown
Contributor

@UebelAndre UebelAndre commented Apr 21, 2021 •
edited
Loading

PR Checklist

Please check if your PR fulfills the following requirements:

  • Does not include precompiled binaries, eg. .par files. See CONTRIBUTING.md for info
  • 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?

  • Bugfix
  • Feature (please, look at the "Scope of the project" section in the README.md file)
  • 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?

I have a .bzl file which loads the requirements.bzl file of a pip_repository and I want to use stardoc to generate documentation it. However, I'm not able to access requirements.bzl because it's not publicly visible outside of the repository.

Issue Number: N/A

What is the new behavior?

The change in this PR exports the file so it can be used for generating documentation.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@google-cla google-cla bot added the cla: yes label Apr 21, 2021
@UebelAndre UebelAndre marked this pull request as ready for review April 21, 2021 03:16
@UebelAndre
Copy link
Copy Markdown
Contributor Author

UebelAndre commented Apr 21, 2021

@hrfuller are you around for a review? 🙏

alexeagle
alexeagle approved these changes Apr 21, 2021
View reviewed changes
Copy link
Copy Markdown
Contributor

@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.

I think this is fundamentally a stardoc bug. This PR fixes one of the many hundreds of places across the Bazel ecosystem where a load() statement can't be traversed statically.

We need to fix stardoc to fail gracefully in this case and leave the referenced symbols undocumented (treat them as some unknown or wildcard type). That's how we fix all the hundreds of issues at once.

That said I'm okay with unblocking you

alexeagle
alexeagle reviewed Apr 21, 2021
View reviewed changes
@UebelAndre
Copy link
Copy Markdown
Contributor Author

UebelAndre commented Apr 21, 2021

I think this is fundamentally a stardoc bug. This PR fixes one of the many hundreds of places across the Bazel ecosystem where a load() statement can't be traversed statically.

We need to fix stardoc to fail gracefully in this case and leave the referenced symbols undocumented (treat them as some unknown or wildcard type). That's how we fix all the hundreds of issues at once.

That said I'm okay with unblocking you

Yeah, there's an effort to address this in stardoc but it continues to be pushed back, it seems bazelbuild/stardoc#69

@alexeagle alexeagle merged commit a4c375b into bazel-contrib:master Apr 21, 2021
@UebelAndre UebelAndre deleted the docs branch April 21, 2021 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexeagle alexeagle alexeagle approved these changes

@brandjon brandjon Awaiting requested review from brandjon

@lberki lberki Awaiting requested review from lberki

Assignees

No one assigned

Labels

cla: yes

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@UebelAndre @alexeagle