-
Notifications
You must be signed in to change notification settings - Fork 432
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
Start building universal wheels #397
Conversation
@@ -1,6 +1,6 @@ | |||
The MIT License | |||
|
|||
Copyright (c) 2010-2011 Stripe (http://stripe.com) | |||
Copyright (c) 2010-2018 Stripe (http://stripe.com) |
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.
This one hadn't been updated in a while ;)
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.
lol. Maybe another seven years until the next time too ...
@@ -0,0 +1,5 @@ | |||
[bdist_wheel] | |||
universal = 1 |
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.
This enables the build of universal wheels by default. Cf. https://packaging.python.org/tutorials/distributing-packages/#universal-wheels
universal = 1 | ||
|
||
[metadata] | ||
license_file = LICENSE |
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.
This is to ensure the license file is included in the wheel files. Cf. https://wheel.readthedocs.io/en/stable/#including-the-license-in-the-generated-wheel-file.
setup.py
Outdated
from version import VERSION | ||
about = {} | ||
with open(os.path.join('stripe', 'version.py')) as f: | ||
exec(f.read(), about) |
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.
This is slightly cleaner than manipulating the sys path, and removes a flake8
warning about imports needing to be grouped at the top of the file. (Note that setup.py
is not automatically validated by flake8 currently.)
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.
Kind of a nit again, but could change about
to something a little more descriptive like version_contents
?
Love the same though. The old code is tremendously opaque by comparison.
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.
👍
|
@brandur-stripe PTAL |
Minor comment above, but LGTM. Having very recently ran into a problem with a package missing from one particular Python version and failing my whole build, this is a pretty exciting change. Thanks OB! |
e143b69
to
aa5c968
Compare
r? @brandur-stripe
cc @stripe/api-libraries
Since we no longer use
2to3
and the library can now directly be used by Python 2 or 3, we also no longer need to build distinct wheels.This PR enables the flag to start building universal wheels by default, as well as add a few opportunistic fixes to the
setup.py
and license files.