-
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
Removed install dependency on setuptools #483
Conversation
Codecov ReportBase: 66.85% // Head: 68.28% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #483 +/- ##
==========================================
+ Coverage 66.85% 68.28% +1.43%
==========================================
Files 12 12
Lines 902 927 +25
==========================================
+ Hits 603 633 +30
+ Misses 299 294 -5
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
The failing tests are due to an old version of setuptools not collecting the licenses as we expect. Maybe I need to add a fallback here. |
Btw: Thanks! |
Curious, I thought a) I still think the correct move here for ideal bootstrapping is to set the package up with flit. Or even have it build itself, perhaps? A custom backend isn't that hard1 when you already have the ability to make wheels. You can hard code the contents, and then you just have to make metadata. Footnotes
|
I got burned before trying to switch to PEP 517 builds. I'm willing to try that again with the |
Not true, the version number does matter. |
It seems to work fine now. I would like someone to look over the changes and give me a thumbs up if it seems a-ok. |
You would have my thumbs up anyway, for fixing that; but I don't know much about Python... looking at https://github.com/pypa/wheel/pull/483/files, from a very formal (non-semantic) perspective, changes seem to be low-invasive except the change to license_paths from bdist_wheel adds a lot of new decision/fallback logic; I think this may cause even more maintenance issues in the future;
My hunch is that license_paths from bdist_wheel is a candidate either for the next cleanup/refactoring, or for future trouble; So I think the change to license_paths from bdist_wheel may or may not be acceptable, but may need future consideration. |
The "old" method, as you saw it, was introduced in the yanked release of v0.38.0. This PR restores backwards compatibility because it can no longer guarantee a specific setuptools version. |
I did some more testing and the new code breaks on setuptools older than v42, but wheel 0.37.1 works with setuptools older than that. I will fix this shortly. |
Unless someone points out any glaring issues in this PR, I'll merge it tomorrow and make a new patch release. |
Fixes #470.