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

Issue 29569630: Noissue - Added flake8-docstrings (sitescripts) (Closed)

Created:
Oct. 9, 2017, 1:32 a.m. by Sebastian Noack
Modified:
Oct. 11, 2017, 5:17 p.m.
Reviewers:
Vasily Kuznetsov
CC:
tlucas
Visibility:
Public.

Description

Noissue - Added flake8-docstrings (sitescripts)

Patch Set 1 #

Total comments: 6
Unified diffs Side-by-side diffs Delta from patch set Stats (+16 lines, -9 lines) Patch
M sitescripts/extensions/bin/createNightlies.py View 1 chunk +1 line, -1 line 3 comments Download
M tox.ini View 4 chunks +15 lines, -8 lines 3 comments Download

Messages

Total messages: 6
Sebastian Noack
https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py#newcode135 sitescripts/extensions/bin/createNightlies.py:135: command = ['hg', 'clone', '-q', self.config.repository, '-u', For some ...
Oct. 9, 2017, 1:56 a.m. (2017-10-09 01:56:17 UTC) #1
Vasily Kuznetsov
LGTM https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py#newcode135 sitescripts/extensions/bin/createNightlies.py:135: command = ['hg', 'clone', '-q', self.config.repository, '-u', On ...
Oct. 9, 2017, 8:39 p.m. (2017-10-09 20:39:47 UTC) #2
Sebastian Noack
https://codereview.adblockplus.org/29569630/diff/29569631/tox.ini File tox.ini (right): https://codereview.adblockplus.org/29569630/diff/29569631/tox.ini#newcode15 tox.ini:15: ignore = D1 On 2017/10/09 20:39:47, Vasily Kuznetsov wrote: ...
Oct. 10, 2017, 4:38 a.m. (2017-10-10 04:38:32 UTC) #3
Vasily Kuznetsov
On 2017/10/10 04:38:32, Sebastian Noack wrote: > https://codereview.adblockplus.org/29569630/diff/29569631/tox.ini > File tox.ini (right): > > https://codereview.adblockplus.org/29569630/diff/29569631/tox.ini#newcode15 ...
Oct. 10, 2017, 10:23 a.m. (2017-10-10 10:23:40 UTC) #4
Sebastian Noack
https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py File sitescripts/extensions/bin/createNightlies.py (right): https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensions/bin/createNightlies.py#newcode135 sitescripts/extensions/bin/createNightlies.py:135: command = ['hg', 'clone', '-q', self.config.repository, '-u', On 2017/10/09 ...
Oct. 10, 2017, 10:01 p.m. (2017-10-10 22:01:48 UTC) #5
Vasily Kuznetsov
Oct. 11, 2017, 5:17 p.m. (2017-10-11 17:17:45 UTC) #6
Message was sent while issue was closed.
On 2017/10/10 22:01:48, Sebastian Noack wrote:
>
https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensi...
> File sitescripts/extensions/bin/createNightlies.py (right):
> 
>
https://codereview.adblockplus.org/29569630/diff/29569631/sitescripts/extensi...
> sitescripts/extensions/bin/createNightlies.py:135: command = ['hg', 'clone',
> '-q', self.config.repository, '-u',
> On 2017/10/09 20:39:47, Vasily Kuznetsov wrote:
> > On 2017/10/09 01:56:17, Sebastian Noack wrote:
> > > For some reason flake8 didn't detect this redundant space before
installing
> > > flake8-docstrings, even though it isn't a D* error.
> > 
> > This is really strange... how are we supposed to trust it after this :/
> 
> I figured out what is going on here. This wasn't related to installing
> flake8-docstrings. Everything works as intended. By default flake8 (and
> pycodingstyles) ignore E24 (along some other errors, i.e. E121, E123, E126,
> E226, E704, W503 and W504). That is because PEP-8 doesn't explicitly
> mention/discourage these practices. However, I went through the list, and
every
> single of these errors is something that you and I would complain about, if
> noticed in a code reviews. So it is good that these are no longer ignored.
> 
> The reason why they are no longer ignored is that we added `ignore = D1` to
the
> flake8 section in tox.ini, overriding any pre-defined ignores. If we wouldn't
> have at least one global ignore in each project (i.e. D1), now, I would
suggest
> to add `ignore =` to each project that doesn't have anything to ignore, in
order
> to get the above errors.

Awesome! Then we can still trust flake8 :D

Powered by Google App Engine
This is Rietveld