-
Notifications
You must be signed in to change notification settings - Fork 61
docs: switching from rtd to book theme and changing logo #253
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
Conversation
WalkthroughSwitched the Sphinx documentation theme from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (3)
🔇 Additional comments (5)
✏️ Tip: You can disable this entire section by setting Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/conf.py (1)
142-145: Remove unsupported mermaid configuration optionmermaid_include_mindmap.Three of these configuration options are supported in sphinxcontrib-mermaid 1.2.3:
mermaid_d3_zoom,mermaid_fullscreen, andmermaid_include_elk. However,mermaid_include_mindmapdoes not appear in the official configuration options for this version and should be removed.
🤖 Fix all issues with AI agents
In `@docs/source/conf.py`:
- Around line 122-125: Remove or clarify the commented-out Sphinx setup snippet:
either delete the three commented lines that define the setup(app: Any) function
and app.add_css_file('my_theme.css') if the custom CSS is no longer needed, or
keep them but replace the commented block with a short explanatory comment
stating why the setup/add_css_file override is preserved for potential future
use.
- Around line 136-137: The LaTeX build will fail because latex_logo and
html_logo point to SVG files; either configure an image converter or use non‑SVG
assets: add the appropriate converter to the Sphinx extensions list (e.g.,
include 'sphinx.ext.imgconverter' and ensure ImageMagick is installed, or
include 'sphinxcontrib.svg2pdfconverter' if you prefer vector PDF conversion) so
Sphinx can convert SVGs for LaTeX, or replace the value of latex_logo (and
optionally html_logo) with a preconverted PNG/PDF file.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (3)
docs/source/assets/logo.svgis excluded by!**/*.svgdocs/source/assets/logo_bottom_text.svgis excluded by!**/*.svguv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
docs/source/conf.pypyproject.tomlrequirements-dev.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: setup and test (macos-latest, 3.13)
- GitHub Check: setup and test (macos-latest, 3.12)
- GitHub Check: setup and test (windows-latest, 3.12)
- GitHub Check: setup and test (windows-latest, 3.13)
🔇 Additional comments (3)
pyproject.toml (1)
52-61: LGTM!The dependency change from
sphinx-rtd-themetosphinx-book-theme>=1.1.3correctly aligns with the theme migration. The version constraint is compatible with the pinned version (1.1.4) inrequirements-dev.txt.docs/source/conf.py (1)
127-135: LGTM!The
sphinx_book_themeconfiguration and options are correctly set up. The repository integration buttons and navigation depth settings are appropriate for the project.requirements-dev.txt (1)
1-4: LGTM!This is an auto-generated lockfile from
uv pip compile. The new dependencies (accessible-pygments,pydata-sphinx-theme,beautifulsoup4,soupsieve) are transitive dependencies brought in bysphinx-book-theme, which builds on top ofpydata-sphinx-theme. The dependency resolution is correct.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
5ae8cb1 to
9ab789d
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.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/source/conf.py (1)
138-141:mermaid_include_mindmapis not a valid sphinxcontrib-mermaid configuration option.Three of the four options are supported:
mermaid_d3_zoom✓mermaid_fullscreen✓mermaid_include_elk✓However,
mermaid_include_mindmapis not a sphinxcontrib-mermaid configuration option. Mindmap is a Mermaid diagram type, not a configuration flag. To use mindmap, ensure yourmermaid_versionsupports it or register the mindmap plugin viamermaid_init_configinstead.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (5)
docs/source/assets/logo.pdfis excluded by!**/*.pdfdocs/source/assets/logo.svgis excluded by!**/*.svgdocs/source/assets/logo_bottom_text.pdfis excluded by!**/*.pdfdocs/source/assets/logo_bottom_text.svgis excluded by!**/*.svguv.lockis excluded by!**/*.lock
📒 Files selected for processing (3)
docs/source/conf.pypyproject.tomlrequirements-dev.txt
🔇 Additional comments (5)
pyproject.toml (1)
52-61: LGTM!The dependency swap from
sphinx-rtd-themetosphinx-book-theme>=1.1.3is consistent with the theme migration inconf.pyand aligns with the resolved version (1.1.4) inrequirements-dev.txt.requirements-dev.txt (1)
1-232: LGTM!This auto-generated lockfile correctly reflects the transitive dependencies of
sphinx-book-theme, includingpydata-sphinx-theme,accessible-pygments,beautifulsoup4, andsoupsieve. The dependency graph comments are properly updated.docs/source/conf.py (3)
64-64: Good addition of image converter extension.Adding
sphinx.ext.imgconverterenables SVG to PDF conversion for LaTeX builds if needed. Note that this requires ImageMagick to be installed on the build system.
123-131: Theme configuration looks correct.The
sphinx_book_themeoptions are well-configured with repository integration buttons and appropriate navbar depth. This aligns with the dependency changes inpyproject.toml.
132-133: Logo files are correctly positioned.Both logo files exist at the specified paths:
docs/source/assets/logo_bottom_text.pdfanddocs/source/assets/logo_bottom_text.svg.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
the nav bar has regressions that will be fixed with the future documentation reorg.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.