[pylint] Fixes all use-maxplit-args, consider-using-enumerate
#12172
Conversation
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
46b7775
to
bb4bd7e
Compare
|
Converting to draft as I need to add tests for the missing coverage |
testing/_py/test_local.py
Outdated
| @@ -207,7 +207,7 @@ def test_visit_norecurse(self, path1): | |||
|
|
|||
| @pytest.mark.parametrize( | |||
| "fil", | |||
| ["*dir", "*dir", pytest.mark.skip("sys.version_info <" " (3,6)")(b"*dir")], | |||
| ["*dir", "*dir", pytest.mark.skip("sys.version_info < (3,6)")(b"*dir")], | |||
There was a problem hiding this comment.
We can remove this:
| ["*dir", "*dir", pytest.mark.skip("sys.version_info < (3,6)")(b"*dir")], | |
| ["*dir", "*dir", b"*dir"], |
There was a problem hiding this comment.
Right ! Removed the reminder for the whole codebase in 7ded8d8
There was a problem hiding this comment.
yikes, we had skip instead of skipif and the tests where broken
There was a problem hiding this comment.
965e942 introduced the issue replacing a pylib basestring with just str
wrong ref used
There was a problem hiding this comment.
Thank you for looking into it, I was stuck. I'm not sure I understand though, should I replace ["*dir", "*dir", b"*dir"], back to ["*dir", "*dir", pytest.mark.skipif("sys.version_info < (3,6)")(b"*dir")], and then fix the tests ? Maybe in another merge request at this point ?
There was a problem hiding this comment.
in the Visitor class of the local path clone, there is a condition we need to replace
https://github.com/pytest-dev/pytest/blob/main/src/_pytest/_py/path.py#L140 - it should also check for bytes
There was a problem hiding this comment.
when asottile took out a lot of cruft a basestring usage was misinterpreted and replaced with just str
unfortunately the skips where wrong and thus the error was never noted as the tests skipped instead of passing
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
9699b3d
to
7ded8d8
Compare
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
7ded8d8
to
5f1012c
Compare
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
9283d4d
to
6f2db17
Compare
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
eb5983b
to
1dbcf8f
Compare
Pierre-Sassoulas
force-pushed
the
more-pylint-fixes
branch
from
1dbcf8f
to
8cb1059
Compare
Pierre-Sassoulas
changed the title
use-maxplit-args, consider-using-enumerate, implicit-str-concatuse-maxplit-args, consider-using-enumerate
|
Thank you @Zac-HD, I was wondering how to cover the line that was not covered but if it's mergeable as is for you I'm going to stop wondering. I'll open a follow-up regarding the problem raised by the implicit-str-concat 👍 |
Using
maxsplit=1fromuse-maxplit-argswill make splits slightly faster and is the important one that will improve performances.The other fix are about readability / style.