-
Notifications
You must be signed in to change notification settings - Fork 477
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Travis CI: Add flake8 tests for syntax errors and undefined names #536
Conversation
For some reason Travis doesn't seem to have noticed this PR. I'll close and reopen it to see if that wakes it up. |
We either need to update all the examples to Python 3 syntax, or exclude the examples from the linting. |
All files pass on Python 2 and the examples directory passes on Python 3. |
Avoid an _undefined name_ by following the Python porting best practice [use feature detection instead of version detection](https://docs.python.org/3/howto/pyporting.html#use-feature-detection-instead-of-version-detection).
Avoid an _undefined name_ by following the Python porting best practice [use feature detection instead of version detection](https://docs.python.org/3/howto/pyporting.html#use-feature-detection-instead-of-version-detection).
Avoid an _undefined name_ by following the Python porting best practice [use feature detection instead of version detection](https://docs.python.org/3/howto/pyporting.html#use-feature-detection-instead-of-version-detection).
Avoid an _undefined name_ by following the Python porting best practice [use feature detection instead of version detection](https://docs.python.org/3/howto/pyporting.html#use-feature-detection-instead-of-version-detection).
All files pass on Python 2 and all files except for the tests directory pass on Python 3. |
try: | ||
text_type = unicode | ||
except NameError: | ||
text_type = str |
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.
I actually prefer the explicit PY3 check here. It's frustrating if we have to make the code worse in my opinion just to satisfy an automated tool. As a compromise, we could add a dependency on six
and just use six.text_type
.
self.allowed_string_types = (basestring, ) | ||
self.linesep = os.linesep | ||
self.write_to_stdout = sys.stdout.write | ||
except NameError: # Python 3 |
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.
I strongly prefer the if PY3
check here. Catching NameError is a bug waiting to happen. If we can't find a way to use flake8 with the explicit if PY3
, I'd rather not use it at all.
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.
To clarify: I'd rather not use it automated in CI. It's still fine to run it manually and fix problems.
g.quit() | ||
try: | ||
g.quit() | ||
except NameError: |
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.
I think the linter identified a real problem with the example here - this fix is just sweeping it under the carpet.
@@ -1,3 +1,6 @@ | |||
# flake8: noqa This file is Python 3-only and async yield is a Py2 syntax error |
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.
Any way to keep linting it on Python 3?
Should I retry with six? Should we just drop the g.quit() lines? |
Add flake8 tests to find Python syntax errors and undefined names.
E901,E999,F821,F822,F823 are the "showstopper" flake8 issues that can halt the runtime with a SyntaxError, NameError, etc. Most other flake8 issues are merely "style violations" -- useful for readability but they do not effect runtime safety.
name
name
in__all__