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

Remove pip._internal.download #7187

Merged
merged 3 commits into from
Oct 18, 2019

Conversation

chrahunt
Copy link
Member

Moves the last of its contents as described in #7145 (comment).

This increases the size of pip._internal.operations.prepare, but now it should be easier to refactor since the not-really-generic functions are alongside their sole caller.

Fixes #6813.

@chrahunt chrahunt added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Oct 13, 2019
@chrahunt chrahunt force-pushed the refactor/move-get-file-content branch 2 times, most recently from 0da5c7a to 7f6c321 Compare October 13, 2019 23:40
@chrahunt chrahunt marked this pull request as ready for review October 14, 2019 00:53
@BrownTruck
Copy link
Contributor

Hello!

I am an automated bot and I have noticed that this pull request is not currently able to be merged. If you are able to either merge the master branch into this pull request or rebase this pull request against master then it will be eligible for code review and hopefully merging!

@BrownTruck BrownTruck added the needs rebase or merge PR has conflicts with current master label Oct 15, 2019
@chrahunt chrahunt force-pushed the refactor/move-get-file-content branch from 7f6c321 to 42ad7a1 Compare October 16, 2019 00:26
@pypa-bot pypa-bot removed the needs rebase or merge PR has conflicts with current master label Oct 16, 2019
@chrahunt chrahunt force-pushed the refactor/move-get-file-content branch from 42ad7a1 to 4f4d7a9 Compare October 16, 2019 00:28
The only user of this module is operations.prepare.RequirementPreparer.
Moving the functionality to the single using module means that
refactoring will be easier (since all the mess is in one place). This
also removes a mis-named module from the top-level of the repository.
@chrahunt chrahunt force-pushed the refactor/move-get-file-content branch from 4f4d7a9 to 5c5c6ec Compare October 17, 2019 01:29
@chrahunt
Copy link
Member Author

Does this need to be split up? Every trivial change to download requires a rebase. 😩

@pradyunsg
Copy link
Member

Ah well... Is someone going to work on the debt that this introduces? If not, I'm inclined to suggest that we leave these separate for now -- we can get rid of the rest of download.py slowly as we refactor things out of it.

@chrahunt
Copy link
Member Author

chrahunt commented Oct 17, 2019

Yes, I plan to - specifically, factoring the hash calculation and download directory handling responsibility out of preparer and the associated functions.

@pradyunsg
Copy link
Member

Awesome! Let's do this then! :)

@pradyunsg pradyunsg merged commit 053346f into pypa:master Oct 18, 2019
@chrahunt chrahunt deleted the refactor/move-get-file-content branch October 18, 2019 01:32
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Nov 17, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Nov 17, 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.

Breakout pip._internal.download to a pip._internal.network sub-package
5 participants