Port to Python 3 #113
Port to Python 3 #113
Conversation
|
cc @glyph :) |
| @@ -77,14 +82,13 @@ def test_findUselessCheckers(self): | |||
| Test for method findUselessCheckers | |||
| """ | |||
| runner = Runner() | |||
| registeredCheckers = sum(runner.linter._checkers.values(), []) | |||
| registeredCheckers = sum(list(runner.linter._checkers.values()), []) | |||
There was a problem hiding this comment.
Do you really need to explicitly call list() in these changes,
or can you leave the calls to list() out? converters like 2to3 tend to put them in, but most times you don't need them.
Otherwise, this patch looks OK.
There was a problem hiding this comment.
On second look, these explicit calls to list() all look needed.
|
Use of the word "also" is a killer :). Can you split this up? Particularly, the verbose pep8=>pycodestyle renaming detracts from trying to look at the rest of the code. |
|
One other minor concern - I'd really like to be able to run twistedchecker locally using pypy to get faster lint times. Right now I can't, due to this unfortunate bug - pylint-dev/pylint#964 - but if we stop testing twistedchecker on 2.7 then this is even less likely to be possible. |
|
D'oh. I suppose you can remove that last comment. I thought I had seen somewhere that this was removing the 2.7 support - but it looks like the tox envs just add 3.5. |
setup.py
Outdated
| @@ -9,7 +9,7 @@ | |||
| setup( | |||
| name='TwistedChecker', | |||
| description='A Twisted coding standard compliance checker.', | |||
| version='0.5.0', | |||
| version='16.0', | |||
There was a problem hiding this comment.
This seems pretty clearly unrelated…
|
Reviewed! @hawkowl - Thank you for taking on the port here. But the change is way too big and does a bunch of unrelated things. Please split this into a series of smaller changes so that each can be reviewed more quickly and landed independently. Particularly, I'd like to see the version bumps taken care of first without the rest of the py3 port involved. |
| "logilab-common == 0.62.0", | ||
| "pep8 == 1.5.7" | ||
| "pylint == 1.5.6", | ||
| "logilab-common == 1.2.1", |
There was a problem hiding this comment.
logilab-common is not needed anymore by pylint ;-)
There was a problem hiding this comment.
Except we use it somewhere, yaaaaaay :P
There was a problem hiding this comment.
This is extra-confusing then. Aren't we using it to import the exception types and such required by pylint? If pylint doesn't use them any more, can we just delete that code?
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
=========================================
+ Coverage 92.21% 92.8% +0.58%
=========================================
Files 27 26 -1
Lines 1169 1098 -71
Branches 193 187 -6
=========================================
- Hits 1078 1019 -59
+ Misses 54 48 -6
+ Partials 37 31 -6
Continue to review full report at Codecov.
|
rodrigc
force-pushed
the
py3
branch
2 times, most recently
from
cf38d69 to
922a469
Compare
Since this needs to run on Python 3, I've removed the Python 3 linter checks -- since it won't be able to run them, and the Twisted codebase is now free of the majority of them.
This also updates to the Py3 compatible version of pylint and newer PEP8.