Skip to content

Conversation

alexeagle
Copy link
Contributor

@alexeagle alexeagle commented Jan 21, 2023

#846 introduced this dependency, and solved the broken examples by installing the dependency only for our own code. However, this was an unintentional breaking change for users, who may not have had bazel_skylib installed in their workspace.
(Note for reviewers, any change to the examples is an indication that it will be breaking to users, as it represents changes required for using the rules)

This effectively reverts #370. At that time we didn't want any dependencies, because managing them under Bazel's WORKSPACE semantics is so difficult for users. Now that bzlmod has reached General Availability in Bazel 6, such dependencies can be managed more easily.

This also allows us to introduce bzl_library calls in our BUILD files, making it less brittle to ensure that users can generate docs for their rules/macros which load from rules_python.

#846 introduced this dependency, and solved the broken examples by installing the dependency only for our own code.
However, this was an inintentional breaking change for users, who may not have had bazel_skylib installed in their workspace.

This effectively reverts #370. At that time we didn't want any dependencies, because managing them under Bazel's WORKSPACE semantics is so difficult for users. Now that bzlmod has reached General Availability in Bazel 6, such dependencies can be managed more easily.

This also allows us to introduce bzl_library calls in our BUILD files, making it less brittle to ensure that users can generate docs for their rules/macros which load from rules_python.
@f0rmiga f0rmiga merged commit bd3a719 into main Jan 21, 2023
@f0rmiga f0rmiga deleted the dep_skylib branch January 21, 2023 01:48
alexeagle added a commit that referenced this pull request Jan 21, 2023
Following #1001 we require that users install bazel-skylib, so we are now free to load from it.
alexeagle added a commit that referenced this pull request Jan 21, 2023
Following #1001 we require that users install bazel-skylib, so we are now free to load from it.
f0rmiga pushed a commit that referenced this pull request Jan 23, 2023
Following #1001 we require that users install bazel-skylib, so we are now free to load from it.
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