Skip to content

Conversation

@xitij2000
Copy link
Contributor

@xitij2000 xitij2000 commented Mar 24, 2023

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

Reviewers:

  • tag reviewer
  • tag reviewer

Merge checklist:

  • All reviewers approved
  • CI build is green
  • Version bumped
  • Changelog record added
  • Documentation updated (not only docstrings)
  • Commits are squashed

Post merge:

  • Create a tag
  • Check new version is pushed to PyPi after tag-triggered build is
    finished.
  • Delete working branch (if not needed anymore)

Author concerns: List any concerns about this PR - inelegant
solutions, hacks, quick-and-dirty implementations, concerns about
migrations, etc.

@openedx-webhooks openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Mar 24, 2023
@openedx-webhooks
Copy link

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.

@xitij2000 xitij2000 changed the title feat: Switch from edx-sphinx-theme to sphinx-book-theme chore: Switch from edx-sphinx-theme to sphinx-book-theme Mar 24, 2023
@xitij2000 xitij2000 force-pushed the kshitij/switch-to-sphinx-book-theme branch from 9ea7a43 to d49c434 Compare March 24, 2023 07:43
@mphilbrick211
Copy link

@xitij2000 - hi there! Looks like some tests will need to be re-run. Thanks!

@xitij2000 xitij2000 force-pushed the kshitij/switch-to-sphinx-book-theme branch from d49c434 to 507d921 Compare March 28, 2023 05:56
@xitij2000
Copy link
Contributor Author

@xitij2000 - hi there! Looks like some tests will need to be re-run. Thanks!

I think the test failures were due to changes in linting rules related to how exceptions are treated. I've reverted unnecessary requirements changes, so the tests are passing now.

@mphilbrick211
Copy link

Hi @bobthebeef! Would you mind reviewing this, and merging if all looks good? Thanks!

@Cup0fCoffee
Copy link

👍

  • I tested this: ran make docs
  • I read through the code
  • Includes documentation

Copy link
Contributor

@feanil feanil left a comment

Choose a reason for hiding this comment

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

1 Change, then I think this is good to go and I can merge.

@xitij2000 xitij2000 force-pushed the kshitij/switch-to-sphinx-book-theme branch 2 times, most recently from b89fe97 to 001b314 Compare April 19, 2023 09:55
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.

See https://github.com/openedx/edx-sphinx-theme/issues/184
@xitij2000 xitij2000 force-pushed the kshitij/switch-to-sphinx-book-theme branch from 001b314 to d4794f4 Compare April 21, 2023 05:27
@feanil feanil merged commit 011634b into openedx:master Apr 21, 2023
@openedx-webhooks
Copy link

@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.

@xitij2000 xitij2000 deleted the kshitij/switch-to-sphinx-book-theme branch April 28, 2023 02:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

open-source-contribution PR author is not from Axim or 2U

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants