-
-
Notifications
You must be signed in to change notification settings - Fork 153
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
PR: Upgrade packaging to avoid legacy/deprecated behavior and follow PEP 517 #272
Conversation
a613178
to
3f7bf6b
Compare
02eb05f
to
091e53e
Compare
091e53e
to
248bec6
Compare
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 @CAM-Gerlach ! I left a suggestion and a comment/question. Besides that this LGTM 👍
Also, thank you @Kant-ain for the misspelling fix suggestion !
248bec6
to
7736341
Compare
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 @CAM-Gerlach for this! I left a couple of comments, otherwise looks good to me.
1f37195
to
e54447d
Compare
7c66e15
to
38f0fe0
Compare
38f0fe0
to
759a350
Compare
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 @CAM-Gerlach for your work on this!
Thanks @CAM-Gerlach !! |
Thanks for your feedback and bearing with me on this @ccordoba12 and @dalthviz ! |
Currently, the package is using the deprecated legacy builder and calling setup.py directly, which is causing warnings and will eventually go away completely. This migrates it to the (more) modern PEP 517 setuptools builder and setup.cfg, with a setup setup.py for backward compatibility. This also makes one change to how the version is handled to avoid having to import the package or execute any dynamic code to get the version statically, which shouldn't affect anything else as
_version
is marked as internal use only (with leading_
). I've compared the wheels and sdists and, aside from the minor updates to the metadata made in a separate commit, they are byte for byte identical.Resolve #270