-
Notifications
You must be signed in to change notification settings - Fork 60
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR! Unfortunately you accidentally removed the one function that's part of the public API. 😄 I also have some simplification suggestions.
try: | ||
CPU_COUNT = max(2, multiprocessing.cpu_count()) | ||
except NotImplementedError: #pragma: no cover | ||
CPU_COUNT = 2 | ||
|
||
|
||
def check(requirements_paths=[], metadata=[], projects=[]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this and its tests removed? This is part of the documented API for the package.
formatted_need = need.format(len(flattened_blockers), | ||
's' if len(flattened_blockers) != 1 else '') | ||
|
||
if encoding == 'utf-8': |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should probably change this to not be so static to UTF-8 as it isn't actually dependent on that encoding, just an encoding that can handle it.
@@ -68,34 +68,47 @@ def projects_from_cli(args): | |||
|
|||
def message(blockers): | |||
"""Create a sequence of key messages based on what is blocking.""" | |||
encoding = getattr(sys.stdout, 'encoding', '') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
encoding = getattr(sys.stdout, 'encoding', '') | |
encoding = getattr(sys.stdout, 'encoding', '').lower() |
formatted_can_port = can_port.format( | ||
'those' if len(flattened_blockers) != 1 else 'that', | ||
len(flattened_blockers), | ||
's' if len(flattened_blockers) != 1 else '', | ||
len(blockers), | ||
'have' if len(blockers) != 1 else 'has', | ||
'their' if len(blockers) != 1 else 'its') | ||
return formatted_need, formatted_can_port | ||
|
||
return [formatted_need, formatted_can_port] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change this from a tuple?
def output_not_on_pypi(not_on_pypi): | ||
lines = [] | ||
|
||
if len(not_on_pypi) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if len(not_on_pypi) > 0: | |
if not_on_pypi: |
for line in output_not_on_pypi(not_on_pypi): | ||
print(line) | ||
|
||
if len(not_on_distlib) > 0: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if len(not_on_distlib) > 0: | |
if not_on_distlib: |
@@ -16,6 +16,7 @@ | |||
|
|||
import distlib.locators | |||
import packaging.utils | |||
import urllib |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this added?
'its transition:') | ||
self.assertEqual(messages[1], want) | ||
|
||
def test_message_with_unknowns(self): | ||
unknowns = [['A']] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for? It doesn't appear used anywhere.
outputs = ciu_main.output_not_on_pypi(unknowns) | ||
self.assertEqual(2, len(outputs)) | ||
want = 'The following 1 project could not be found on pypi.org:' | ||
self.assertEqual(outputs[0], want) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to be so strict on the output. Just check that the string appears somewhere in the output.
self.assertEqual(outputs[0], want) | |
self.assertIn(want, outputs[0]) |
Same with the other output tests.
@@ -65,5 +65,4 @@ class NetworkTests(unittest.TestCase): | |||
def test_supports_py3(self): | |||
self.assertTrue(pypi.supports_py3("caniusepython3")) | |||
self.assertFalse(pypi.supports_py3("pil")) | |||
# Unfound projects are considered ported. | |||
self.assertTrue(pypi.supports_py3("sdfffavsafasvvavfdvavfavf")) | |||
self.assertEqual(pypi.supports_py3("sdfffavsafasvvavfdvavfavf"), None) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.assertEqual(pypi.supports_py3("sdfffavsafasvvavfdvavfavf"), None) | |
self.assertIsNone(pypi.supports_py3("sdfffavsafasvvavfdvavfavf")) |
@proinsias closing this due to inactivity in responding to the PR review. If you manage to get back to this I can re-open the PR. |
Closes #131 and #213.