Blacken pulp_python #177
Blacken pulp_python #177
Conversation
|
Hello @werwty! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on June 01, 2018 at 18:58 Hours UTC |
| metadata_version = serializers.CharField( | ||
| help_text=_('Version of the file format') | ||
| help_text=_( | ||
| "The type of the distribution package " "(e.g. sdist, bdist_wheel, bdist_egg, etc)" |
| required=False, | ||
| allow_blank=True, | ||
| help_text=_( | ||
| "The maintainer's name at a minimum; " "additional contact information may be provided." |
| required=False, | ||
| allow_blank=True, | ||
| help_text=_( | ||
| "The Python version(s) that the distribution is guaranteed to be " "compatible with." |
| **digest) | ||
| python_models.DistributionDigest.objects.create( | ||
| project_specifier=specifier, **digest | ||
| ) |
There was a problem hiding this comment.
I'm not sure I like how it does this. It makes sense in a bunch of places but it feels wrong here.
| _( | ||
| "Either the 'repository' or 'repository_version' need to be specified " | ||
| "but not both." | ||
| ) |
| 'project_packages': package_detail_data | ||
| }) | ||
| with open(metadata_relative_path, "w") as simple_metadata: | ||
| context = Context({"project_name": name, "project_packages": package_detail_data}) |
There was a problem hiding this comment.
This looks nicer on two lines
| @@ -73,7 +73,8 @@ def _fetch_inventory(version): | |||
| inventory = set() | |||
| if version: | |||
| for content in python_models.PythonPackageContent.objects.filter( | |||
| pk__in=version.content).only("filename"): | |||
| pk__in=version.content | |||
| ).only("filename"): | |||
| }) | ||
| self.assertEqual(len(page['results']), 1) | ||
| page = self.client.get(PYTHON_PUBLISHER_PATH, params={"name": self.publisher["name"]}) | ||
| self.assertEqual(len(page["results"]), 1) |
There was a problem hiding this comment.
A lot of these tests look better on two lines
|
XD we're too opinionated for black And it does try to format this line https://github.com/pulp/pulp_python/pull/177/files#diff-27efe7f2ffa6f1561afb34c9ffa703faR257 It was worth a try, but black's probably not quite right for us right now |
|
I'm not sure how I feel about it. I agree with some of the things it did and disagree with a lot of others. I don't really like the idea of formatting the smash tests because I want to be able to easily compare them against the file smash tests. |
|
TBF we can point it only to pulp_python/app and not have it touch the smash tests. |
|
I wouldn't mind taking "the good suggestions" and making a formatting PR for them |
|
That's not a bad idea, I can modify this PR to be "good" formatting only. |
|
I think at a minimum we can autoformat outright PEP8 errors, and I know tools for that exist. |
|
Wrong button sorry |
No description provided.