Deleted legacy pip_import rule #582
Conversation
|
@alexeagle @hrfuller Based on the conversation at #436 (comment), I've opened this PR. I think the rules have been communicated as EOL for some time so just deleting them feels acceptable to me but if you're hoping for a more transitional closure this code then I'm happy to defer to a PR from either one of you 😄 |
| @@ -37,6 +37,7 @@ | ||
| /bazel-genfiles | ||
| /bazel-out | ||
| /bazel-testlogs | ||
| user.bazelrc |
There was a problem hiding this comment.
This is a recommended pattern and was used in testing this Pr. I'd recommend it stay if possible
| @@ -1 +1,2 @@ | ||
| test --test_output=errors | ||
| # https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file | ||
| try-import %workspace%/user.bazelrc |
There was a problem hiding this comment.
This change seems good but also unrelated to this PR. Should it be its own?
There was a problem hiding this comment.
Does it matter hugely if this is included? I was a way for me to test the changes since the examples are already a little difficult to run. I found this to be necessary in validating the changes were safe and would opt to leave it in.
There was a problem hiding this comment.
It doesn't matter hugely. In cases where we have to revert functionality it can matter, because this is a feature we want regardless of this PR.
|
@hrfuller i think the changes here should be merged as is. Do you feel super strongly otherwise? |
PR Checklist
Please check if your PR fulfills the following requirements:
.parfiles. See CONTRIBUTING.md for infoPR Type
What kind of change does this PR introduce?
What is the current behavior?
As described in #436 (comment), maybe it's time to clean out the old pip repository rules. In investigating cache invalidation issues. I've often been confused by what code path was being used since the two are quite similar.
Issue Number: #436 (comment)
What is the new behavior?
This change deletes some old rules that have long since been considered legacy and have modern replacements that are actively being developed.
Does this PR introduce a breaking change?
Other information