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

Revert "Add option to use "pip download" instead of "pip wheel" to do… #808

Merged
merged 1 commit into from
Aug 27, 2022

Conversation

groodt
Copy link
Collaborator

@groodt groodt commented Aug 27, 2022

…wnload wheels for other platforms (#773)"

This reverts commit 6a43ebd.


Exploring if this PR is causing main to fail. See: https://buildkite.com/bazel/rules-python-python/builds/3242#0182dec5-050b-4fa3-9506-5e606ad0c4e1

Update: Reverting this PR will fix main. I think I should have updated the PR branch before merging because it was a bit old (25 days).

…wnload wheels for other platforms (#773)"

This reverts commit 6a43ebd.
@UebelAndre
Copy link
Contributor

@groodt #809 contains the fix for CI, would it be possible to merge this instead of reverting?

@groodt groodt merged commit 4999f63 into main Aug 27, 2022
@groodt groodt deleted the revert-773-cross-platform-wheel-download branch August 27, 2022 20:14
@groodt
Copy link
Collaborator Author

groodt commented Aug 27, 2022

@UebelAndre I prefer to revert, review the fix, merge the fix if good and then revert the revert.

@UebelAndre
Copy link
Contributor

@groodt So with #809 merged could this now be reverted? Didn't the passing CI on that PR show it addressed the issues this revert was trying to undo?

UebelAndre added a commit to UebelAndre/rules_python that referenced this pull request Aug 27, 2022
@UebelAndre
Copy link
Contributor

I've created #810 to hopefully show the changes in #809 address the issues uncovered by the original implementation.

@groodt
Copy link
Collaborator Author

groodt commented Aug 27, 2022

@UebelAndre I did similar in #811

Can you close #810 so all the context is self-contained?

@UebelAndre
Copy link
Contributor

Can you close #810 so all the context is self-contained?

Sorry, figured it'd be more convenient for me to open it so you could be the reviewer (not needing another rules_python maintainer to come around for the approval). The PR is closed.

@groodt
Copy link
Collaborator Author

groodt commented Aug 28, 2022

Sorry, figured it'd be more convenient for me to open it so you could be the reviewer (not needing another rules_python maintainer to come around for the approval). The PR is closed.

No worries. Thanks! I should have been more clear that I intended to do it. I was just unwinding the PRs. The PR should merge shortly and I'll tag a new release in the next few days as well.

ianpegg-bc pushed a commit to ianpegg-bc/rules_python that referenced this pull request May 12, 2023
bazelbuild#808)

Revert "Add option to use "pip download" instead of "pip wheel" to download wheels for other platforms (bazelbuild#773)"

This reverts commit 6a43ebd.
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.

5 participants