Skip to content

Conversation

cjerdonek
Copy link
Member

This is a follow-on to PR #6835 that finishes removing base_command.py's dependency on index.py / PackageFinder, PipSession, and pip_version_check, etc.

This is again related to #4768 ("Speedup startup time").

@cjerdonek cjerdonek added type: refactor Refactoring code skip news Does not need a NEWS file entry (eg: trivial changes) labels Aug 7, 2019
@cjerdonek cjerdonek force-pushed the trim-base-command-imports branch 2 times, most recently from e3774ed to 2e970c5 Compare August 7, 2019 21:15
@cjerdonek cjerdonek force-pushed the trim-base-command-imports branch from 2e970c5 to 505456f Compare August 7, 2019 22:18
Copy link
Member

@pradyunsg pradyunsg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It'd be handy to add tests to verify that handle_pip_version_check is called in the correct scenarios.

@cjerdonek cjerdonek force-pushed the trim-base-command-imports branch from ed8082e to 9b1ece3 Compare August 8, 2019 06:15
@cjerdonek
Copy link
Member Author

It'd be handy to add tests to verify that handle_pip_version_check is called in the correct scenarios.

@pradyunsg Okay, I added some tests like this as you requested. I added--

  1. A test of the base Command class in test_base_command.py, checking that handle_pip_version_check() is indeed called by command.main(), and

  2. Tests of each of the IndexGroupCommand commands (download, install, list, and wheel), testing whether handle_pip_version_check() calls pip_version_check(), for each of the possible combinations of values of options.disable_pip_version_check and options.no_index.

@cjerdonek cjerdonek force-pushed the trim-base-command-imports branch from 7163ddd to 234ef43 Compare August 8, 2019 12:37
@cjerdonek cjerdonek merged commit c9a043b into pypa:master Aug 9, 2019
@cjerdonek cjerdonek deleted the trim-base-command-imports branch August 9, 2019 00:04
@pradyunsg
Copy link
Member

Hah! You're awesome @cjerdonek!

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Sep 8, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 8, 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.

2 participants