Closed
Conversation
3e85e3f to
0687635
Compare
12 tasks
Collaborator
Author
|
Closing. Proceeding with smaller and less extreme approach here. #807 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is a fairly large refactor and unfortunately some breaking changes. The intent here is to reduce complexity and duplication.
Historically, we had 2 rules to install packages from PyPI (
pip_installandpip_parse), withpip_installbeing the original. It worked well, but had a significant flaw in that it would eagerly fetch and install all packages. This was slow and frustrating when there were a large number of dependencies, sopip_parsewas born!pip_parse(even though it is still a repository rule) lazily fetched and installed packages. This was adopted by most of the community based on evidence from issues raised, the bazel Slack channels and the rule maintainers.Maintaining both versions should no longer be necessary. There was a lot of duplication in both versions and it required extra effort and consideration for adding any new features.
I've taken the approach (probably 🌶️ ) that
pip_installis a better name, so I've introduced this change where bothpip_installandpip_parseshare the same code, but with a small shim to handle therequirements_lockvsrequirementsdifference in argument. I think a deprecation log message should be added and then pip_parse should be removed.2 Breaking Changes:
requirement()macro to reference dependencies. The naming style of the external repositories has changed, so this will require asedacross any.bazelfiles if these users upgrade. This isn't a big deal in my opinion.