-
Notifications
You must be signed in to change notification settings - Fork 294
fix: Don't error if no new wheels were built #1086
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
Conversation
joerick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. A test would also be good, you could add an integration test in the test_0_basic module.
|
Hi @joerick, I'm not entirely sure of the cause of this test failure - any thoughts? https://app.travis-ci.com/github/pypa/cibuildwheel/jobs/567099381#L339-L359 |
|
Looks like a flaky test/random failure. I've restarted it. |
joerick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Brilliant, thanks! A few things to tweak and we can get this in 🙂
…low-empty is active)
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
for more information, see https://pre-commit.ci
…Small style tweak
5da78c3 to
abedb68
Compare
|
@joerick all requested changes made - over to you! |
joerick
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Thank you!
Howdy, this PR contains a simple fix for an issue which I came across when using the
--allow-emptyflag. There is some logic incibuildwheel.util.print_new_wheelswhich is raising an error if no new wheels were built.Details
With this PR:
Details
Let me know if you'd like anything adjusted (e.g. the message that is printed). I'm also happy to add an integration test if you could point me to the right place to add it.
Thanks!