Skip to content
This repository was archived by the owner on Oct 21, 2022. It is now read-only.

Conversation

@liyanhui1228
Copy link
Member

@liyanhui1228 liyanhui1228 commented Aug 27, 2018

Fix #65

This PR also includes some refactoring of the existing code which moves some functions to common utils file.

@liyanhui1228 liyanhui1228 changed the title [WIP] Implement the deprecated dependency finder Implement the deprecated dependency finder Aug 28, 2018
package_name)
deprecated_deps = []

for dep_name in dependency_info.keys():

Choose a reason for hiding this comment

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

Maybe just "for dep_name in dependency_info:"

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

"OutdatedDependency<'pip', HIGH_PRIORITY>",
"OutdatedDependency<'ply', HIGH_PRIORITY>")

self._store = mock.Mock()

Choose a reason for hiding this comment

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

Maybe it is better to pass in an autospec, otherwise it is possible that you have a typo in your method name, and the test still pass.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

finder = deprecated_dep_finder.DeprecatedDepFinder()

self.assertEqual(finder.py_version, '3')
self.assertTrue(

Choose a reason for hiding this comment

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

assertIsInstance is available after Python 3.2. I am not sure what Python versions should we support, but definitely use that if it is possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@ylil93 ylil93 left a comment

Choose a reason for hiding this comment

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

LGTM

@ylil93 ylil93 merged commit b6240b5 into GoogleCloudPlatform:master Sep 4, 2018
@liyanhui1228 liyanhui1228 deleted the deprecated_dep_finder branch September 4, 2018 17:53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants