-
Notifications
You must be signed in to change notification settings - Fork 525
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
docs: replace sphinx-rtd-theme with sphinx-book-theme #4266
docs: replace sphinx-rtd-theme with sphinx-book-theme #4266
Conversation
Compared to `sphinx-rtd-theme`, `sphinx-book-theme` is a clean and modern Sphinx theme. Generated by the task: njzjz-bot/njzjz-bot#11.
📝 Walkthrough📝 WalkthroughWalkthroughThe pull request includes updates to the Sphinx documentation configuration in Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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: 1
🧹 Outside diff range and nitpick comments (1)
doc/conf.py (1)
Line range hint
46-52
: Remove commented out extensions block.This commented block contains duplicate configuration that could become outdated and confusing. Consider removing it since the active extensions are properly configured below.
-# extensions = [ -# 'recommonmark', -# "sphinx_book_theme", -# 'myst_parser', -# 'sphinx_markdown_tables', -# 'sphinx.ext.autosummary' -# ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- doc/conf.py (3 hunks)
- pyproject.toml (1 hunks)
🔇 Additional comments (2)
doc/conf.py (1)
56-56
: LGTM: Theme configuration looks correct.The changes properly configure the new sphinx-book-theme by:
- Adding it to the extensions list
- Setting it as the html_theme
The configuration follows Sphinx conventions where package names use hyphens (sphinx-book-theme) and Python module names use underscores (sphinx_book_theme).
Also applies to: 178-178
pyproject.toml (1)
100-100
: Verify compatibility with major version update.The
deepmodeling-sphinx
dependency has been updated from>=0.1.0
to>=0.3.0
. Since this is a major version bump in a pre-1.0 package, it might contain breaking changes.Let's check the changelog and dependencies:
for more information, see https://pre-commit.ci
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
Signed-off-by: Jinzhe Zeng <jinzhe.zeng@rutgers.edu>
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
🧹 Outside diff range and nitpick comments (1)
doc/_static/css/custom.css (1)
Line range hint
6-9
: Consider making the logo width responsive.The fixed width of 275px might not adapt well to smaller screens. Consider using relative units or adding a max-width constraint.
.wy-side-nav-search .wy-dropdown > a img.logo, .wy-side-nav-search > a img.logo { - width: 275px; + width: min(275px, 100%); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- doc/_static/css/custom.css (1 hunks)
🔇 Additional comments (1)
doc/_static/css/custom.css (1)
Line range hint
14-21
: LGTM! Theme detection improvement.The switch from media queries to data-theme attributes is a good improvement as it:
- Allows manual theme switching independent of system preferences
- Aligns with the new sphinx-book-theme implementation
Let's verify the dark logo exists:
✅ Verification successful
Dark mode logo path verified successfully
The dark mode logo file exists at the expected path relative to the CSS file location, confirming that the content URL reference in the CSS will work correctly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the dark logo file exists # Expected: Find the dark logo file referenced in the CSS # Test: Check if the dark logo exists relative to the CSS file location fd --base-directory doc "_static" --type f --exec test -f "doc/_static/logo-dark.svg" && echo "✓ Dark logo exists" || echo "✗ Dark logo missing"Length of output: 130
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #4266 +/- ##
==========================================
+ Coverage 84.34% 84.35% +0.01%
==========================================
Files 549 549
Lines 51463 51463
Branches 3051 3051
==========================================
+ Hits 43404 43410 +6
+ Misses 7097 7095 -2
+ Partials 962 958 -4 ☔ View full report in Codecov by Sentry. |
Compared to
sphinx-rtd-theme
,sphinx-book-theme
is a clean and modern Sphinx theme.Generated by the task: njzjz-bot/njzjz-bot#11.
Summary by CodeRabbit
New Features
Bug Fixes