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

[pyreverse] updated docs for filter-mode #9806

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

kidq330
Copy link

@kidq330 kidq330 commented Jul 16, 2024

Type of Changes

Type
πŸ› Bug fix
✨ New feature
πŸ”¨ Refactoring
βœ“ πŸ“œ Docs

Description

Improved clarity for pyreverse help docs after some trouble with usage, more details in linked issue

Closes #9805

@kidq330 kidq330 requested a review from DudeNr33 as a code owner July 16, 2024 19:18
Copy link

codecov bot commented Jul 16, 2024

Codecov Report

All modified and coverable lines are covered by tests βœ…

Project coverage is 95.79%. Comparing base (6236b91) to head (023e927).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #9806   +/-   ##
=======================================
  Coverage   95.79%   95.79%           
=======================================
  Files         174      174           
  Lines       18902    18902           
=======================================
  Hits        18107    18107           
  Misses        795      795           
Files Coverage Ξ”
pylint/pyreverse/main.py 93.75% <ΓΈ> (ΓΈ)

@Pierre-Sassoulas Pierre-Sassoulas added Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry labels Jul 16, 2024

This comment has been minimized.

Copy link
Contributor

πŸ€– According to the primer, this change has no effect on the checked open source code. πŸ€–πŸŽ‰

This comment was generated for commit 023e927

Copy link
Collaborator

@DanielNoord DanielNoord left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution!

@@ -0,0 +1,3 @@
Improved clarity for pyreverse help docs after some trouble with usage, more details in linked issue
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this. I'll add the correct label so CI still passes.

except constructor
'OTHER' filter protected and private
attributes""",
"help": "filter attributes and functions according to <mode>. Correct modes are:"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why did you decide to remove the formatting here?

@DanielNoord DanielNoord added Skip news πŸ”‡ This change does not require a changelog entry and removed Skip news πŸ”‡ This change does not require a changelog entry labels Jul 17, 2024
Copy link
Collaborator

@DudeNr33 DudeNr33 left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution!
The addition to the help message of the 'OTHER' option value is helpful.

Maybe you have already figured it out, but in the issue you mention that -f SPECIAL_A does what you want - that is not entirely true; it is just a side effect. SPECIAL_A is not a valid option value (hence the error message you see).
pyreverse, in that case, does not crash but also not simply stick to the normal default value (PUB_ONLY), instead it simply ignores the value. If SPECIAL_A was the only value passed to this option, this is then equivalent to ALL (not filter at all).

'OTHER' filter protected and private
attributes""",
"help": "filter attributes and functions according to <mode>. Correct modes are:"
"'PUB_ONLY' - filter all non public attributes;"
Copy link
Collaborator

Choose a reason for hiding this comment

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

PUB_ONLY is the default, and thus we should not remove this information from the help message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[pyreverse] Dunder methods in diagrams
4 participants