8000 `requirements.bzl` is now visible outside `pip_repository`s by UebelAndre · Pull Request #463 · bazel-contrib/rules_python · GitHub
[go: up one dir, main page]
Skip to content

requirements.bzl is now visible outside pip_repositorys #463

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 21, 2021
Merged

requirements.bzl is now visible outside pip_repositorys #463

merged 2 commits into from
Apr 21, 2021

Conversation

UebelAndre
Copy link
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
Contributor Author

@hrfuller are you around for a review? 🙏

alexeagle
alexeagle approved these changes Apr 21, 2021
View reviewed changes
Copy link
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
Contributor Author

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
0