Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Issue 29992563: Noissue - Work around N812 and E117 from flake8 update (Closed)

Created:
Jan. 29, 2019, 4:32 p.m. by Vasily Kuznetsov
Modified:
Jan. 31, 2019, 12:30 p.m.
Reviewers:
rhowell
Base URL:
https://hg.adblockplus.org/python-abp
Visibility:
Public.

Description

Noissue - work around N812 errors from flake8 update Repository: https://hg.adblockplus.org/python-abp Base revision: 199cda9f1098

Patch Set 1 #

Total comments: 2

Patch Set 2 : Ignore errors instead of working around the linter, avoid E117 #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+4 lines, -4 lines) Patch
M abp/filters/rpy.py View 1 1 chunk +2 lines, -2 lines 1 comment Download
M tests/test_render_script.py View 1 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 4
Vasily Kuznetsov
Hi Rosie! While reviewing your other commit I have noticed that the "flake8" environment currently ...
Jan. 29, 2019, 4:37 p.m. (2019-01-29 16:37:24 UTC) #1
rhowell
Hi Vasily! I noticed this issue too, thanks for fixing it! Just one comment, see ...
Jan. 29, 2019, 8:28 p.m. (2019-01-29 20:28:40 UTC) #2
Vasily Kuznetsov
Hi Rosie, Thanks for the suggestion, I have used it. Meanwhile pycodestyle released an update ...
Jan. 30, 2019, 10:42 a.m. (2019-01-30 10:42:48 UTC) #3
rhowell
Jan. 30, 2019, 4:18 p.m. (2019-01-30 16:18:47 UTC) #4
On 2019/01/30 10:42:48, Vasily Kuznetsov wrote:
> Hi Rosie,
> 
> Thanks for the suggestion, I have used it.
> 
> Meanwhile pycodestyle released an update that generated another error -- I
have
> fixed that one too.
> 
> Cheers,
> Vasily
> 
>
https://codereview.adblockplus.org/29992563/diff/29992564/tests/test_render_s...
> File tests/test_render_script.py (right):
> 
>
https://codereview.adblockplus.org/29992563/diff/29992564/tests/test_render_s...
> tests/test_render_script.py:35: SimpleHTTPServer, SocketServer = server,
> socketserver
> On 2019/01/29 20:28:39, rhowell wrote:
> > Just curious, why not do it in 2 lines instead of 3?  E.g.
> > 
> >     from http import server as SimpleHTTPServer  # noqa: N812
> >     import socketserver as SocketServer  # noqa: N812
> > 
> > This way, we're more explicit in acknowledging the linter error, we save a
> line,
> > and it seems _slightly_ more readable to me. This method also seems
consistent
> > with how some other popular projects handle similar situations (e.g.
> >
>
https://github.com/pypa/pip/blob/master/src/pip/_internal/utils/hashes.py#L14).
> 
> I was trying to avoid disabling the linter. But all in all, my code is working
> around the linter anyway, so we might as well be honest and mark it
explicitly.
> I'll go with your version.
> 
> https://codereview.adblockplus.org/29992563/diff/29993565/abp/filters/rpy.py
> File abp/filters/rpy.py (right):
> 
>
https://codereview.adblockplus.org/29992563/diff/29993565/abp/filters/rpy.py#...
> abp/filters/rpy.py:70: # The condition is a Python 2/3 way of saying "unicode
> string".
> Reformatted this to avoid E117.

LGTM

Powered by Google App Engine
This is Rietveld