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

Improve pyreverse documentation #10063

Merged
merged 7 commits into from
Nov 11, 2024

Conversation

Julfried
Copy link
Contributor

@Julfried Julfried commented Nov 3, 2024

Type of Changes

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

Description

Follow up to 10045

Copy link

codecov bot commented Nov 3, 2024

Codecov Report

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

Project coverage is 95.80%. Comparing base (ac2ae53) to head (9891da4).
Report is 8 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #10063   +/-   ##
=======================================
  Coverage   95.79%   95.80%           
=======================================
  Files         174      174           
  Lines       18940    18962   +22     
=======================================
+ Hits        18144    18166   +22     
  Misses        796      796           

see 3 files with indirect coverage changes

@Julfried Julfried mentioned this pull request Nov 3, 2024
@Pierre-Sassoulas Pierre-Sassoulas added Documentation πŸ“— Skip news πŸ”‡ This change does not require a changelog entry labels Nov 3, 2024
Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

I pushed the change I thought of (easier done than explained). Would you mind checking the result @DanielNoord ?

@DanielNoord
Copy link
Collaborator

I like it, but we need to tweak the toctree slightly:
image

When clicking on Filtering and Scope I remain on the Basic Usage location in the navigation. Can we fix that?

@Julfried
Copy link
Contributor Author

Julfried commented Nov 5, 2024

I think this is intentional, because these sections are all on the same page. For me, if I click on Filtering and Scope, it just scrolls to the according section.

@DanielNoord
Copy link
Collaborator

Yes, but shouldn't it be another dropdown? They are on a different level than "Basic Usage" section wise, so it shouldn't be on the same level I think?

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Nov 5, 2024

I don't know if we can do anything about it, isn't it a bug or feature from Sphinx auto generated toc ? (Moving them one level down, that we can do)

:height: 1452
:alt: Package diagram generated by pyreverse
:align: center

.. toctree::
:maxdepth: 1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we tweak this?

Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to currently, we managed to do it for the dev guide so I should be able to.

Copy link
Member

Choose a reason for hiding this comment

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

This is surprisingly hard. Might have to change the headings underline again.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have a possible solution that fixes both the heading hierarchy and improves the documentation readability. Would you like me to create a commit with these changes?

Copy link
Member

Choose a reason for hiding this comment

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

Sure @Julfried, thank you for proposing (I tried to increase maxdepth without success)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Here is my idea. One thing I noticed: this table of contents collapses when the window is narrow on smaller screens. Maybe this could be fixed in the theme settings. What do you think?

Copy link
Member

@Pierre-Sassoulas Pierre-Sassoulas left a comment

Choose a reason for hiding this comment

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

I think the current version is good enough ℒ️ (especially with the mini-toc created manually in Usage) Let me know what you think @DanielNoord :)

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!

@DanielNoord DanielNoord merged commit 614f80e into pylint-dev:main Nov 11, 2024
26 checks passed
@Julfried Julfried deleted the restructure-documentation branch November 11, 2024 14:48
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.

3 participants