Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

Change appropriate "Nested" definitions into "Inaccessible", and add new codes #561

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

thejcannon
Copy link
Contributor

Some devs want to be able to ignore inaccessible definitions, and some might want to not. This should allow them the option to choose.

(This also helps keep the definition of is_public pure and straightforward and correct according to the documentation)

Please make sure to check for the following items:

  • Add unit tests and integration tests where applicable.
  • Add a line to the release notes (docs/release_notes.rst) under "Current Development Version".

Copy link
Contributor Author

@thejcannon thejcannon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I couldn't find a good term after scouring python.org for what this might be called. I wanted to avoid "nested" because that exists for classes. "Inner" also didn't feel correct because methods are inside classes. I landed on "inaccessible" because the caller can't access them.

src/tests/test_cases/test.py Outdated Show resolved Hide resolved
@@ -183,6 +183,7 @@ class ConfigurationParser:
)
BASE_ERROR_SELECTION_OPTIONS = ('ignore', 'select', 'convention')

DEFAULT_IGNORE = {"D121", "D123"}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other tools have this kind of "we have the ability to report these codes, but their off by default" functionality. I figured this belonged to that category. Note if people are selecting ignores today, this will no longer be the default.

src/pydocstyle/parser.py Outdated Show resolved Hide resolved
src/pydocstyle/parser.py Outdated Show resolved Hide resolved
src/pydocstyle/parser.py Outdated Show resolved Hide resolved
@thejcannon
Copy link
Contributor Author

@samj1912 this should be ready to review 😄

@thejcannon
Copy link
Contributor Author

@samj1912 got time for a quick review?

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

Successfully merging this pull request may close these issues.

2 participants