[DEPR]: edx-sphinx-theme

[feanil]

Proposal Date

29 November 2022

Target Ticket Acceptance Date

13 December 2022

Earliest Open edX Named Release Without This Functionality

Palm - 2023-04

Rationale

• There is no maintainer for this theme.
• We've decided to start using an off-the-shelf them for Open edX docs(sphinx-book-theme)
• Maintaining our own theme adds more maintenance burden.

Removal

This repository https://github.com/openedx/edx-sphinx-theme/ will be archived after doing one more PyPI release which will indicate that the project is deprecated and no longer maintained.

Replacement

Anyone building docs should use the sphinx-book-theme instead.

Deprecation

• Release theme with deprecation warnings. openedx-unsupported/edx-sphinx-theme#190
• Review the sphinx-book-theme for a11y issues that are too large to deal with docs.openedx.org#230
• [How-To, Documentors] Write a guide to migrating from the edx-sphinx-theme to the sphinx-book-theme docs.openedx.org#231
• Check/Update cookiecutters openedx-unsupported/edx-sphinx-theme#193

Migration

People can switch to the sphinx-book-theme by adding the sphinx-book-theme to their requirements and updating their sphinx conf.py to use the sphinx-book-theme.

Additional Info

Discourse Post: https://discuss.openedx.org/t/deprecation-removal-edx-sphinx-theme/8817

[robrap]

Thanks @feanil. I think this is great. I wonder if we could crowdsource a doc for migrating (unless you already have that). For example, are there settings that no longer apply, or should be renamed, or new ones that are useful to know about, etc.

[feanil]

That's a great idea Robert, I think we can collect any items we want to cover on this ticket and then turn it into a "How to Migrate from edx-sphinx-theme to sphinx-book-theme" doc, what do you think?

[robrap]

Also, for additional "Rationale", the old theme had a bunch of styling bugs, including being broken on mobile. See "Fix Sphinx theme" in https://openedx.atlassian.net/wiki/spaces/AC/pages/2624553266/Developer+Docs+Framework+Notes, if you need more details.

[jmbowman]

I agree this is a good idea, just want to make sure we've covered two of the reasons for edx-sphinx-theme's existence that weren't mentioned in the description:

• Way back in the early days, one of the original reasons for forking the theme was that we urgently needed to make some a11y fixes. Are we reasonably sure that the new theme is sufficiently accessible?
• Later we added a link for providing doc feedback via a Google Form that could be used even by people unfamiliar with rST and GitHub. Does the new theme support anything comparable, and/or do we want to retain that functionality?

[robrap]

@jmbowman: It’s good to have that background:

1. I say we review the replacement for a11y and help fix it if that is needed.
2. For most of the time the Google Doc existed, it was a black hole that collected comments that no one ever looked at. Instead, we could have a how-to on all the ways someone could provide feedback.

[robrap]

I’d clarify that maintenance of a custom theme either adds maintenance cost, or leads to a badly broken theme if it is neglected, which was our case.

[feanil]

@jmbowman good context on the a11y work, I didn't know about that. I've added openedx/docs.openedx.org#224 to review the a11y of the new theme and we can update/make improvements based on that but a quick tab-through and keyboard navigation test seems to show no major issues for now. If we do find some in a more thorough test I think it makes sense to fix forward upstream.

[feanil]

As for the feedback form, there is a new feedback form, there is a new feedback form that Jenna and I are monitoring and it's on all the new pages:

[feanil]

Given that there were no concerns on the discourse post and all concerns on this issue have been addressed. This deprecation is now accepted.

[feanil]

I've added a bunch of sub-tasks that need to be completed before this theme can be considered deprecated.

[MaxFrank13]

I've noticed that the update to sphinx 6.0.0 has a breaking change involving the use of jquery see the changelogs.
We use jquery in the edx-sphinx-theme repo here.

I believe the version will need to be pinned at 5.3.0 until the deprecation is complete.

[robrap]

Something is odd about the current state of the requirements. Sphinx is supposedly constrained to 1.8.5 here in edx-sphinx-theme. Typically we include constraints for base.in requirements in the package, but that doesn't seem to be happening here, since edx-platform is at 5.3.0 for sphinx.

I wonder if our code to publish constraints is missing this due to a difference in capitalization in base.in (Sphinx vs sphinx), or some other reason?

[jmbowman]

No new edx-sphinx-theme versions have been published since July 1, 2021. That predates our fix to include constraints.txt information in the package metadata, which was made on November 9, 2021.

[robrap]

Note: @jmbowman created openedx-unsupported/edx-sphinx-theme#197 to handle this constraint issue. This should happen before the repo is archived.

[robrap]

@feanil: Did we ever end up with a how-to for this (see earlier comment)? I noticed @xitij2000 is implementing much of this work, and it would be great for reviewers if decisions were captured in a doc.

For example, I noticed the following changes:

- copyright = edx_theme.COPYRIGHT  # pylint: disable=redefined-builtin
- author = edx_theme.AUTHOR
+ copyright = f'{datetime.now().year}, edX Inc.'  # pylint: disable=redefined-builtin
+ author = 'edX Inc.'

I'm guessing we just went with the old settings, but is edX Inc. what we actually want here?

[xitij2000]

@robrap The how-to can be found here: https://docs.openedx.org/en/latest/developers/how-tos/switch-to-the-sphinx-book-theme.html

As for the copyright etc. That is a good point. I copied over the values from edx-sphinx-theme, however as you pointed out perhaps they should be updated to say "The Axim Collaborative"

@feanil I know you've updated the value in html_theme_options. Should I update these values for copyright and author for all open PRs?

[feanil]

@xitij2000 yea, that's a good catch, let's do that. The Author should be Axim Collabortive and the same for the Creative-Commons licensensing section.

[robrap]

Thanks @xitij2000 and @feanil.

[robrap]

@xitij2000: I'd propose adding a link to this DEPR and to the how-to in your PR and commit comments. Merged PRs could potentially get the PR description updated as well for context, but I'll leave that to you all to decide.

[xitij2000]

Yea, I think we should migrate them to the correct theme anyway and remove the old one. Sounds good about not making more progress till next sprint. Let me know when that starts so I don't bother you before then :-)

I've created PRs for the final batch and they have been reviewed.

[feanil]

@xitij2000 I think we have 3 repos left, you've got PRs in progress for credentials and blockstore it looks like, I just want to make sure you have edx-platform on your radar as well? Also, FYI, I'll be on vacation next week so I'll not be responding to reviews then. I'll be back on June 12th.

[xitij2000]

@xitij2000 I think we have 3 repos left, you've got PRs in progress for credentials and blockstore it looks like, I just want to make sure you have edx-platform on your radar as well? Also, FYI, I'll be on vacation next week so I'll not be responding to reviews then. I'll be back on June 12th.

Yes, it is on my radar, just pushed that PR as well.

[robrap]

I noticed the old theme when going to https://edx.readthedocs.io/projects/edx-toggles/en/latest/how_to/documenting_new_feature_toggles.html. I guess we don't have a step to ensure that changes have actually made it out to readthedocs, or was this repo missed?

[feanil]

The repo was not missed it looks like: openedx/edx-toggles#261

So likely an issue with the publishing of that specific repo, looks like it's failing to build due to requirements issues with the RTD setup: https://readthedocs.org/projects/edx-toggles/builds/20898194/

[robrap]

Thanks @feanil. I put up openedx/edx-toggles#278 to fix the build.

[feanil]

The edx-sphinx theme has been removed from all openedx repositories. The last change left is to update common-constraints(openedx/edx-lint#349) and then we can close this ticket and archive this repository.