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

Refactor utils.py into a utils module to reduce code complexity and prune out unused utils. #4992

Closed
matteius opened this issue Mar 18, 2022 · 1 comment
Assignees
Labels
Category: Development Issue affects development workflow. PR: merged The PR related to this issue has been merged. Type: Enhancement 💡 This is a feature or enhancement request.

Comments

@matteius
Copy link
Member

Placeholder for news item for PR Ref: #4990

The issue

The pipenv/utils.py has grown to 2338 lines of code, which makes it unwieldy and disorganized. When trying to consider larger bugs and refactors this is problematic because of the logical complexity of navigating around and understanding what kind of util you are looking at. I would like to do something similar with core.py at some point as well, but this is my first iteration of proposing reorg'ing some of the code.

The fix

Move the existing utils.py code into a utils module organized by functionality type. This is a first pass at it, and I suspect there may be some tweaking to my naming conventions to make everyone happy. The only draw back I see to doing this is loosing some of the git annotations around this code, but ultimately that would happen whenever we get around to organizing the code better and I think good code organization is more important than legacy annotations which would still ultimately be in the git history.

The only modifications to code being moved are the following methods have been dropped due to lack of use:

escape_grouped_arguments + unit tests
parse_python_version + unit tests
add_to_set
sys_version
get_indexes_from_requirement
interrupt_handled_subprocess
rmtree
Also two unused code lines were removed pipenv/core.py because I had already run the test suite showing they were not used after my IDE tipped me off about that.

@matteius matteius self-assigned this Mar 18, 2022
@matteius matteius added Type: Enhancement 💡 This is a feature or enhancement request. PR: awaiting-review The PR related to this issue is awaiting review by a maintainer. labels Mar 18, 2022
@matteius matteius added the Category: Development Issue affects development workflow. label Mar 19, 2022
@oz123 oz123 removed the PR: awaiting-review The PR related to this issue is awaiting review by a maintainer. label Mar 29, 2022
@matteius
Copy link
Member Author

PR was merged to main.

@matteius matteius added the PR: merged The PR related to this issue has been merged. label Mar 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Category: Development Issue affects development workflow. PR: merged The PR related to this issue has been merged. Type: Enhancement 💡 This is a feature or enhancement request.
Projects
None yet
Development

No branches or pull requests

2 participants