chore: Switch from edx-sphinx-theme to sphinx-book-theme

openedx / edx-documentation

http://docs.edx.org

Other

161 stars 333 forks source link

chore: Switch from edx-sphinx-theme to sphinx-book-theme #2126

Closed xitij2000 closed 1 year ago

xitij2000 commented 1 year ago

The edx-sphinx theme is being deprecated, and replaced with sphinx-book-theme. This removes references to the deprecated theme and replaces them with the new standard theme for the platform.

Testing instructions:

Run make docs and see that the docs are generated properly and use the sphinx-book-theme

See https://github.com/openedx/public-engineering/issues/200

openedx-webhooks commented 1 year ago

Thanks for the pull request, @xitij2000! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

Cup0fCoffee commented 1 year ago

:+1:

[x] I tested this: built docs and verified that it's using the new theme
[x] I read through the code

Cup0fCoffee commented 1 year ago

@xitij2000 Is the Build Docs check failing due to warnings? You could try to add the titles to the files that are showing the warnings to test.

xitij2000 commented 1 year ago

@feanil I was having trouble understanding why this was making tests fail, so I dug a bit deeper, and it seems that since the new theme displays a sidebar with links, it triggers these warnings which causes the tests to fail.

The issue was two pages that are included in the index but don't include a title. I am not sure why these pages exist. If it is to maintain backwards-compatible links, then new new commit I've added will break those links. Otherwise, I feel removing those pages shouldn't have an effect, since without a title they wont show up in the sidebar or in search.

Alternatively, I can add a title to those pages, but I'm not sure what that would be.

feanil commented 1 year ago

@xitij2000 that seems reasonable to me.

openedx-webhooks commented 1 year ago

@xitij2000 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.