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

Improvements & general refactoring #133

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

Conversation

AA-Turner
Copy link

@AA-Turner AA-Turner commented Jan 30, 2025

To allow python/cpython#129120, we need to introduce the ability to have a different URL base for social card images to the canonical URL.

In preparation to start this work I have made some general improvements to the codebase, which are described in each commit. I would suggest rebase-merging or fast-forward merging to maintain understandable history.


4388f9e git mv setup.py pyproject.toml
173cfc8 Use declarative metadata

Closes #44. Move from the deprecated setup.py to the standardised pyproject.toml, maintaining git history by git mving the file first. We also fix a markup error in LICENCE.md, as it is declared to be a markdown file.

6cf22f2 Support READTHEDOCS_CANONICAL_URL

Closes #129. Read the Docs no longer inject html_baseurl.

07ae701 Declare support for Python 3.13

Closes #131. Python 3.13 was released in October last year.

4be7f53 Improve performance of get_title()

We don't need to call the method twice, we can just return both strings and use tuple unpacking.

3f99ead Define a social_cards optional extra

CLoses #112. This makes the feature more discoverable.

d400791 Use pathlib in tests

Closes #111. sphinx.testing.path is deprecated.

c161663 Sort imports

Linting.

2cff296 Improve performance of get_description()

We use a membership test with in, so we should use a set rather than a list.

e416bc5 Drop support for Python 3.8
c75cafb Drop support for Sphinx 5

Reduces the support matrix. Python 3.8 is end-of-life. I would also suggest dropping Python 3.9, but that is more arguable. Sphinx 5 was released three years ago.

4328637 Adopt Ruff
7f3c66a Enable Ruff linters

Ruff is a fast linter and formatter that has wide adoption, including in Sphinx and CPython.

26cfaf4 Improve type annotations

Type annotations are useful in IDEs such as PyCharm, and for downstream projects.

d78b198 Factor out social_card_for_page()
117abcb Fix config type in get_tags()
43d177c Avoid passing the app object around
a5e22d3 Factor out read_the_docs_site_url()

Miscellaneous refactoring to make functions smaller and arguments more explicit.

8c7a7cf Enable more linter categories in Ruff

Enable more categories beyond general correctness, including requiring that every function is annotated and ensuring imports aren't redundantly aliased (fixing e.g. import docutils.nodes as nodes).


cc @ItayZiv @TheTripleV @Daltz333 @sciencewhiz

A

@AA-Turner AA-Turner force-pushed the refactor branch 2 times, most recently from eff1e85 to 8b0e920 Compare January 30, 2025 21:50
@AA-Turner
Copy link
Author

cc @ItayZiv @TheTripleV @Daltz333 @sciencewhiz anything I can do to help with getting this PR reviewed?

@TheTripleV
Copy link
Member

Ope forgot to reply, sorry...

Thanks for all the work you've put into this.

For context:
This org (wpilibsuite)'s primary objective is to develop WPILib, "the standard software library provided for teams to write code for their FIRST Robotics Competition (FRC) robots". That's a high school robotics competition where kids CAD, build, and program robots.

We started writing Sphinx extensions soon after we adopted Sphinx for our documentation.
Out of these, Opengraph for embeds and Rediraffe for anti-404 have taken a life of their own and are used by the community way more than I ever thought they would be.

At this time, I only have the bandwidth to fully support this extension for the needs of frc-docs, which are currently met.
The last time I merged without thoroughly reviewing a PR, I broke CPython's CI doc builds :(.

Would the Sphinx org (or sphinx-contrib) want to take ownership of this repo? That way, more people currently active in developing Sphinx extensions could be added as maintainers.

Copy link
Member

@TheTripleV TheTripleV left a comment

Choose a reason for hiding this comment

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

I didn't test this against frc-docs but it looks fine to me

@AA-Turner
Copy link
Author

AA-Turner commented Apr 1, 2025

Thanks for the reply/context! Very understandable, thank you for all the work you've put in so far.

Would the Sphinx org (or sphinx-contrib) want to take ownership of this repo?

I'd be happy to transfer this repo to @sphinx-doc, yes. Please could you add me as a collaborator/owner on this repo so that I can move it, and please could you add me on PyPI/read the docs? My username is the same everywhere.

If the same applies to rediraffe, we'd also be happy to maintain this. Let me know who should retain commit/publish permissions.

A

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

Successfully merging this pull request may close these issues.

2 participants