Skip to content
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

Create a new wheel_builder module and move WheelBuilder+friends #7288

Merged
merged 4 commits into from
Nov 5, 2019

Conversation

pradyunsg
Copy link
Member

Moves WheelBuilder and friends into a dedicated module.

These are already basically independent from the rest of pip._internal.wheel so doing so makes a lot of sense IMO. Reduces ~500 lines from wheel.py.

Refactor done w/ PyCharm's refactoring tools + cleanup.

@pradyunsg pradyunsg added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Nov 3, 2019
@chrahunt
Copy link
Member

chrahunt commented Nov 3, 2019

This is a good thing to move out IMO. I would also move the tests.

@pradyunsg
Copy link
Member Author

This is already a pretty huge PR as is, so I'm weary of adding more things to it. I'd prefer to move tests in a follow up.

@pradyunsg
Copy link
Member Author

Actually, you're right -- there's only 1 test for WheelBuilder (ahahahahahaha) and that's failing already. I'll move that into it's own file, as a reminder that we need to expand these tests.

@sbidoul
Copy link
Member

sbidoul commented Nov 3, 2019

@pradyunsg it would be cool if #7285 could go be merged before, as it's my second attempt and the previous one was also killed by a refactoring underneath it. 🙏

@pradyunsg
Copy link
Member Author

pradyunsg commented Nov 3, 2019

@sbidoul Yep yep -- that's on my radar. Don't worry, I don't plan on merging this change before that PR. :)

Edit: To be clear, even if I do, I don't expect there to be any major issues or merge conflicts with this PR (a git merge master or git rebase master should be all that's needed to get upto speed).

@pradyunsg pradyunsg force-pushed the refactor/new-wheel_builder-module branch from 9f10b82 to 9eb4352 Compare November 3, 2019 14:42
@pradyunsg pradyunsg force-pushed the refactor/new-wheel_builder-module branch from 9eb4352 to b1f2f74 Compare November 4, 2019 06:22
@pradyunsg
Copy link
Member Author

Updated after #7285 merged. :)

@chrahunt chrahunt merged commit 7d59679 into pypa:master Nov 5, 2019
@pradyunsg pradyunsg deleted the refactor/new-wheel_builder-module branch November 5, 2019 09:05
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Dec 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Dec 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation skip news Does not need a NEWS file entry (eg: trivial changes) type: refactor Refactoring code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants