Fix up khan-linter to be python3-only. #42
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Now that we're on python3 everywhere, we don't need `six` anymore. Or the `vendored/py2` directory. I also changed all our shebang lines to say `python3` explicitly, rather than `python`. This isn't really needed, but some folks don't even have a /usr/bin/python on their machine, only /usr/bin/python3! And we might as well be clear. While testing this, I discovered a few lint errors that are new to new versions of python, that I cleaned up. I also discovered that the vendored pyflakes we have is too old for modern pythons, so I updated it, and also mccabe while I was at it. Issue: https://khanacademy.slack.com/archives/C02NMB1R5/p1710546934835669 Test plan: ./runlint_test.py
MiguelCastillo
approved these changes
Mar 29, 2024
| @@ -1,5 +1,3 @@ | |||
| #!/usr/bin/env python | |||
There was a problem hiding this comment.
good catch! Presumably some refactor removed main and forgot to clean this up. Did you need to chmod? Im assuming it had the binary bit given it had a shebang.
| @@ -158,7 +155,7 @@ def _extended_fnmatch_compile(pattern): | |||
|
|
|||
| # If the code below this line has horrible syntax highlighting, check | |||
| # this out: http://stackoverflow.com/questions/13210816/sublime-texts-syntax-highlighting-of-regexes-in-python-leaks-into-surrounding-c | |||
| _METACHAR_RE = re.compile(r'[[*?!]') | |||
| _METACHAR_RE = re.compile(r'[*?![]') | |||
There was a problem hiding this comment.
hard to tell which way is more readable.
| @@ -822,7 +819,7 @@ def main(files_and_directories, | |||
| except Exception: | |||
| _get_logger().error(u"ERROR linting %r with %s:\n%s" % ( | |||
| files, type(lint_processor), | |||
| traceback.format_exc().decode('utf-8'))) | |||
| traceback.format_exc())) | |||
There was a problem hiding this comment.
I cant keep track of what needs .decode('utf-8') anymore. 😅
|
@csilvers let me know if Im blocking you from getting this one out the door :) |
Nope! I'm just using it locally for a while to make sure there are no hidden problems before I land it. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
Now that we're on python3 everywhere, we don't need
sixanymore. Or thevendored/py2directory.I also changed all our shebang lines to say
python3explicitly,rather than
python. This isn't really needed, but some folks don'teven have a /usr/bin/python on their machine, only /usr/bin/python3!
And we might as well be clear.
While testing this, I discovered a few lint errors that are new to new
versions of python, that I cleaned up. I also discovered that the
vendored pyflakes we have is too old for modern pythons, so I updated
it, and also mccabe while I was at it.
Issue: https://khanacademy.slack.com/archives/C02NMB1R5/p1710546934835669
Test plan:
./runlint_test.py