-
Notifications
You must be signed in to change notification settings - Fork 29
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
Add qiskit
theme based on Furo
#425
Conversation
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.
looking good so far, just left some comments about the documentation side of things
@@ -54,7 +54,7 @@ jobs: | |||
rm -rf example_docs/_build_legacy | |||
- name: Build Furo theme | |||
run: | | |||
THEME=_qiskit_furo tox run -e docs | |||
THEME=qiskit tox run -e docs |
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.
Why's this tox syntax different from the examples you provide in the contributing guide, which don't use run
?
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.
Tox 4 recommends using run
for disambiguation. It's not actually necessary with our environments, but I did it in CI a while ago. But with CONTRIBUTING.md, it's annoying to type more characters so I kept it simple.
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.
LGTM, just one comment for the readme 🚀
Unblocks #232. With this PR, users will be able to set
html_theme = "qiskit"
to use Furo.This PR adds instructions about how to migrate.
It also switches the default theme in
qiskit_sphinx_theme
's example docs toqiskit
. 95% of my work this month has been on Furo, so that's a better default.Reminder:
qiskit_ecosystem
theme upcomingOnce we add the Ecosystem theme, only Qiskit itself should use the
qiskit
theme. In the meantime, everyone should migrate fromqiskit_sphinx_theme
toqiskit
(barring migration issues).