search

avocado-framework / inspektor

Inspektor code checker
Other
11 stars 17 forks source link

Disable w503 and update to version 0.2.3 #24

Closed will-Do closed 7 years ago

will-Do commented 7 years ago

Recently a new merged file in avocado-vt introduced a W503 complain that 'line break before binary operator', and then all the follow PRs are failed on Travis CI build. But We do should disable it by default. A refer from PEP8: https://www.python.org/dev/peps/pep-0008/#should-a-line-break-before-or-after-a-binary-operator

will-Do commented 7 years ago

@lmr Please help to review and merge this PR, thanks.

ldoktor commented 7 years ago

Well the proper fix would be to address those issues in avocado-vt sources, wouldn't it? We already did this in Avocado and it is fairly simple change which improves the readability.

As a temporary fix I could recommend using inspekt --disable E501,E265,W503,W601,E402 in your check script.

will-Do commented 7 years ago

@ldoktor Yes, I tried to address it in avocado-vt first, but after do some search, I think W503 should be disabled, you can check the commit message of the first patch.

will-Do commented 7 years ago

BTW, Travis CI just run 'make check', and the script we should update it selftests/checkall, I don't think it is the right or temporary way to fix this problem.

ldoktor commented 7 years ago

I think it is as avocado uses custom setting as well (we ignore naked Exceptions). Anyway I'd be against, but you might convince other maintainers... (my goal would be to disable all of them except the bare minimum)

ldoktor commented 7 years ago

https://github.com/avocado-framework/avocado/blob/master/selftests/checkall

will-Do commented 7 years ago

Guess you just convinced me :) The main propose of this fix is disable W503, and if you agree with that, I will close this one and raise a new PR for avocado-vt. BTW, just think about your goal and I agree with it, let the checker do his own job and custom setting for each project who using it.

ldoktor commented 7 years ago

OK, thanks, closing this PR.

ldoktor commented 7 years ago

OK I'm sorry for denying this. After re-reading the topic I believe this could be applied without the version increase as it's not necessary needed. (basically the W503 is disabled by default on newer versions, but some older versions keep enforcing it)