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

Added bzl_library targets for use in external repos using stardoc #936

Closed
wants to merge 1 commit into from
Closed

Conversation

UebelAndre
Copy link

What type of PR is this?

Feature

What package or component does this PR mostly affect?

other

What does this PR do? Why is it needed?

This PR exposes bzl_library targets.

Which issues(s) does this PR fix?

This fixes the ability to use stardoc in other projects that depend on bazel-gazelle

Other notes for review
This implements the same thing as #760 but with a fix for the visibility of the top level //:bzl_srcs target so it can actually be used in other projects.

Also, I'm unsure if bazel-contrib/rules_go#2621 can be applied to this repo as well.

@UebelAndre UebelAndre requested a review from jayconrod as a code owner October 17, 2020 16:49
@jayconrod
Copy link
Contributor

Sorry, I'd rather not merge this. It's a duplicate of #760. I'll comment on that one.

@jayconrod jayconrod closed this Oct 19, 2020
@UebelAndre
Copy link
Author

@jayconrod Why would you close this PR (one that works) in favor of an older one that's had no activity for months?

@jayconrod
Copy link
Contributor

This one isn't correct either. At a quick glance, it adds a duplicate dependency in WORKSPACE, and it doesn't follow conventions from @bazel_skylib//gazelle/bzl. I don't want to spend time reviewing duplicate PRs.

@UebelAndre
Copy link
Author

Ultimately I don't care which PR gets merged but I disagree with closing this over an inactive PR. Hopefully #760 gets merged soon though 🤞.

Thanks for taking a look 🙏

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

Successfully merging this pull request may close these issues.

2 participants