Forcibly disabling noqa should be optional

jayvdb commented 9 years ago

While I can sympathise with your decision to disable pep8.noqa (https://github.com/flowroute/ebb-lint/blob/da34049b11fa93f7da967995b0bf593c36a71d60/ebb_lint/flake8.py#L23) , this flake8 plugin would be more useful if that could be disabled.

For example, a large and old codebase being modernised needs to disable certain flake8 codes either globally across the codebase or individually in some instances for various reasons (such as backwards compatibility; a corner-case bug in the linter; etc) and obviously disabling a code for single instances is much better for the codebase than not having the rule being checked at all.

This could be enabled by default, and only disabled using a configuration item in tox.ini. I'd be happy to submit a PR if you're interested, so I can add your plugin to the list in https://github.com/wikimedia/pywikibot-core/blob/master/tox.ini#L31 , and disable only the codes that we can't easily fix immediately.

habnabit commented 9 years ago

I still feel like this applies:

The feature is over-used and over-general; it's not possible to only mark certain errors as acceptable, and as a result, it's possible for a line marked noqa for one error to completely mask a different error.

A better alternative would be something like rust's lint attributes. For example, a comment like # flake8:allow(E123).

jayvdb commented 9 years ago

I don't disagree, and even appreciate having this anti-noqa feature, but on non-trivial codebase it sometimes needs to be disabled. Unfortunately there is not a good path forward, until flake8/pep8 allows more precision with noqa. The code review team I am most engaged gets very picky whenever noqa's are being introduced, wanting to ensure there is no alternative way to silence the flake code which is not less ugly.

Here is an example of the problem, on a codebase I can "do anything (sane)" with. Disabling noqa means I need alternatives for the two noqa's in setup.py

N812 : setuptools really does have a class named test , and py.test's docs recommend renaming it (and I agree; ugly should be removed as early as possible - in the imports). I suppose I could work around that by confusing pep8-naming:

from setuptools.command.test import test

TestCommand = test

The N812 rule is a good one, generally, but occasionally it needs to be ignored. If the line only has one import, per pep8, then noqa cant suppress other error/style problems.

delayed import: import pytest. Again, this follows the py.test documentation linked above. Obviously this can not be moved to the top of the module, I do not want to suppress this rule everywhere, and I do not want to exclude setup.py from linting.

habnabit commented 9 years ago

I'm not opposed to being able to toggle off the linter on specific lines for specific codes, but only if it was possible to be more specific. ebb-lint already changes the way that flake8 handles long lines and noqa, so it doesn't seem like it would be much of a stretch to add in more specific exceptions. I can't remember how deeply you'd have to reach into flake8 to do this.

jayvdb commented 9 years ago

I need to play a bit more, but I think I can build a better 'no qa' like functionality sufficient for my needs using https://github.com/jayvdb/flake8-putty , in which case your deactivation of pep8's noqa is less of a concern to me.

jordal commented 8 years ago

This project is now maintained elsewhere. Since I'm unaware of any github feature to move an issue across repositories, please recreate your issue under the new maintainer.

habnabit commented 8 years ago

https://github.com/habnabit/ebb-lint/issues! Didn't realize this was off by default.

flowroute / ebb-lint

Forcibly disabling noqa should be optional #7