-
Notifications
You must be signed in to change notification settings - Fork 187
Conversation
def _expand_error_codes(code_parts): | ||
"""Returns expanded set of error codes to ignore.""" | ||
codes = set(ErrorRegistry.get_error_codes()) | ||
epanded_codes = set() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be expanded_codes
(missing the x
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Fixed.
epanded_codes = set() | ||
|
||
for part in code_parts: | ||
if len(part) < 4: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should an error be raised if this is false?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not entirely sure how it would reach such a state, but tried catching a TypeError here. Let me know if I'm not understanding something.
f4dd3ad
to
a3e29ae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add docs to the new CLI interface and update the release notes.
|
||
@staticmethod | ||
def _expand_error_codes(code_parts): | ||
"""Returns expanded set of error codes to ignore.""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The moment when your docstring wouldn't pass insepction by the tool that it's in :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤦 Would have thought the tests would catch that...
@@ -324,6 +335,23 @@ def foo(): | |||
assert 'D103' not in err | |||
|
|||
|
|||
def test_wildcard_add_ignore_cli(env): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a negative test (e.g., trying to pass --add-ignore=D1034
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added another test. Let me know if it's not what you meant.
@@ -16,6 +16,7 @@ Major Updates | |||
New Features | |||
|
|||
* Decorator-based skipping via ``--ignore-decorators`` has been added (#204). | |||
* Support for using pycodestyle style wildcards has been added (#209). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also specify the original issue (#72).
@@ -42,6 +42,14 @@ Usage | |||
regular expression; default is --ignore-decorators='' | |||
which does not ignore any decorated functions. | |||
|
|||
.. note:: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A shorter version of this should also be added the to flag description (add it in the code, then call pydocstyle --help
and paste the result above). It should be succinct, e.g., replace "with a list of comma separated error codes" with "with a list of comma separated error codes or prefixes".
Merged. Thanks! |
An attempt at implementing the request in #72
Suggestions on how to succinctly document this in the help output would be appreciated.