-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Move final wheel builder copy operation to wheel command #7517
Move final wheel builder copy operation to wheel command #7517
Conversation
Also, pluralize variable names for readability and consistency with similar variables in callers.
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.
Technically this changes the behavior so that wheels will appear after all builds instead of as they are built, however:
- I think that this will be necessary going forward anyway with the new resolver work, since we won't know until the end which wheels are relevant, and will have to build some wheels in the middle of resolving
- the wheels that we do succeed in building will be available in the cache, so if an issue does happen the next invocation of the command should be fast since it will take files directly from the cache instead of rebuilding them
So LGTM
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.
LGTM, though this should be called out in the release notes.
1ef342d
to
1f39950
Compare
I added a .feature news explaining the new behavior. I suppose it's not a significant change that needs to be called out as breaking. |
@chrahunt I can continue with moving the FYI I tried removing the unpacking, but it's still necessary for WheelDistribution.get_pkg_resources_distribution, which in turn gets called by |
Yup, sounds good to me. Regarding |
Removes one more non-build concern from
WheelBuilder.build()
.