Fixing flake8 in travis and flake8 errors #3019
Conversation
|
@daviddavis, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bmbouter, @asmacdo and @dralley to be potential reviewers. |
| @@ -20,8 +20,8 @@ before_script: | |||
| - psql -U postgres -c "CREATE USER pulp WITH SUPERUSER LOGIN;" | |||
| - psql -U postgres -c "CREATE DATABASE pulp OWNER pulp;" | |||
| script: | |||
| # flake8 the diff | |||
| - "git diff HEAD^ '*.py' | flake8 --diff --config flake8.cfg" | |||
There was a problem hiding this comment.
We originally did it this way because most of pulp failed flake8. I'm happy with this change assuming this passes when merged.
platform/pulp/app/tasks/importer.py
Outdated
| @@ -38,7 +37,7 @@ def sync(repo_name, importer_name): | |||
| """ | |||
| importer = models.Importer.objects.get(name=importer_name, repository__name=repo_name).cast() | |||
| if not importer.feed_url: | |||
| raise ValueError_("An importer must have a 'feed_url' attribute to sync.") | |||
| raise ValueError(_("An importer must have a 'feed_url' attribute to sync.")) | |||
There was a problem hiding this comment.
Thank you for this! You just saved me some time!
pulp-dev.py
Outdated
| @@ -362,7 +362,7 @@ def install(opts): | |||
| os.system('chmod 3775 /var/cache/pulp') | |||
There was a problem hiding this comment.
Pretty sure this whole file is unused in Pulp 3. Want to remove in a separate commit on the same PR?
There was a problem hiding this comment.
As evidence, this Python 2 would have failed.
| @@ -1,3 +1,5 @@ | |||
| # TODO: fix flake8 undefined name errors on this file | |||
There was a problem hiding this comment.
You can remove this since you ignored server/*
| @@ -6,7 +6,7 @@ | |||
| import celery | |||
|
|
|||
| from pulp.common import tags | |||
| from pulp.common.error_codes import PLP0002, PLP0003 | |||
| from pulp.common.error_codes import PLP0002 | |||
There was a problem hiding this comment.
Undo this change as well. I think we should not add any changes to server/* even to make it flake8. It might imply that this code is not dead in the 3.0-dev line.
rel-eng/lib/tools.py
Outdated
| @@ -35,7 +35,7 @@ def increment(version): | |||
|
|
|||
| def main(): | |||
Error: $ git diff HEAD^ '*.py' | flake8 --diff --config flake8.cfg fatal: ambiguous argument '*.py': unknown revision or path not in the working tree. Use '--' to separate paths from revisions, like this: 'git <command> [<revision>...] -- [<file>...]' The command "git diff HEAD^ '*.py' | flake8 --diff --config flake8.cfg" exited with 0.
platform/pulpcore/tasking/tasks.py
Outdated
| @@ -64,7 +64,7 @@ def _queue_reserved_task(name, inner_task_id, resource_id, inner_args, inner_kwa | |||
| concurrently with yours. | |||
| inner_args (tuple): The positional arguments to pass on to the task. | |||
| inner_kwargs (dict): The keyword arguments to pass on to the task. | |||
| options (dict): For all options accepted by apply_async please visit: http://docs.celeryproject.org/en/latest/reference/celery.app.task.html#celery.app.task.Task.apply_async #NOQA | |||
| options (dict): For all options accepted by apply_async please visit: http://docs.celeryproject.org/en/latest/reference/celery.app.task.html#celery.app.task.Task.apply_async # NOQA | |||
There was a problem hiding this comment.
since we use lowercase #noqa elsewhere, it would be better to keep it consistent.
Error:
Also, I recommend against running flake8 against the diff instead of all files. Doing so may not pick up some flake8 errors. For example, if a function call gets removed and it's the last one in the file, then the unused import warning won't get raised as the import line isn't part of the diff.