Skip to content

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

Merged
groodt merged 1 commit into
mainfrom
revert-773-cross-platform-wheel-download
Aug 27, 2022
Merged

Revert "Add option to use "pip download" instead of "pip wheel" to do…#808
groodt merged 1 commit into
mainfrom
revert-773-cross-platform-wheel-download

Conversation

@groodt

@groodtgroodt commented Aug 27, 2022

Copy link
Copy Markdown
Contributor

…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
Copy Markdown
Contributor

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

@groodt
groodt merged commit 4999f63 into mainAug 27, 2022
@groodt
groodt deleted the revert-773-cross-platform-wheel-download branch August 27, 2022 20:14
@groodt

Copy link
Copy Markdown
ContributorAuthor

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

@UebelAndre

Copy link
Copy Markdown
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

Copy link
Copy Markdown
Contributor

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

@groodt

Copy link
Copy Markdown
ContributorAuthor

@UebelAndre I did similar in #811

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

@UebelAndre

Copy link
Copy Markdown
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
Copy Markdown
ContributorAuthor

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.

Sign up for freeto 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

@groodt@UebelAndre@f0rmiga@alexeagle@hrfuller