Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ignore-names does not apply for all checks #43

Open
igorraush opened this issue Aug 22, 2017 · 5 comments
Open

ignore-names does not apply for all checks #43

igorraush opened this issue Aug 22, 2017 · 5 comments

Comments

@igorraush
Copy link

igorraush commented Aug 22, 2017

Unless I'm missing something, the ignore argument to the visitor methods is used only in FunctionNameCheck (N802). Is this by design? Would be great to apply this option for all checks.

@igorraush igorraush changed the title --ignore-names does not apply for all checks ignore-names does not apply for all checks Aug 22, 2017
@sigmavirus24
Copy link
Member

I'd merge a pull request that implements this. I suspect the reasoning is that the implementor didn't want --ignore-names to be overly broad. Perhaps we should deprecate that option in favor of ignore-function-names and ignore-class-names etc. so that people can be specific.

@igorraush
Copy link
Author

@sigmavirus24 Judging by the first comment in #19, the implementer submitted a partial WIP, intending to apply the logic to all error types. That said, I do agree with you that type-specific whitelists would be useful. So you'd like to continue supporting ignore-names with the current behavior but start emitting a DeprecationWarning, and implement ignore-*-names? I'd be happy to submit a PR.

@igorraush
Copy link
Author

igorraush commented Aug 27, 2017

Another discussion point: how granular should the options be? Ideally, there should be an option to ignore a list of names for each error type. One way to do it:

  • ignore-function-names (applies to N802)
  • ignore-class-names (applies to N801)
  • ignore-variable-names (applies to N803, N806, N81x)
  • ignore-Nxx-names (applies to Nxx specifically)

Another approach entirely (similar to something suggested in the original PR) is to support

--ignore-names X N803:Y N804:mcs`

which would suppress all errors about X (class, function, etc.), N803 about Y, etc.

What are your thoughts?

@sigmavirus24
Copy link
Member

We don't have a reverse mapping of violation code to checker class and I'd rather not build one or attempt to maintain it. Using explicitly named flags without using weird magical syntax makes the most sense. It's simple, explicit, and can be easily configured in INI.

@jparise
Copy link
Member

jparise commented Jan 28, 2019

ignore-names now also applies to N806, N815, and N816 checks as of #96.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants