Skip to content
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

Use sphinx_rtd_theme directly instead of vendoring it #1501

Closed
gregmuellegger opened this issue Jul 30, 2015 · 5 comments
Closed

Use sphinx_rtd_theme directly instead of vendoring it #1501

gregmuellegger opened this issue Jul 30, 2015 · 5 comments
Assignees
Labels
Improvement Minor improvement to code

Comments

@gregmuellegger
Copy link
Contributor

Are there any reasons why the sphinx rtd theme is copied into the RTD code base instead of referenced as dependency?

There were bugs like #1497 that would not have been raised if the theme would have been up to date.

@ericholscher
Copy link
Member

ha, read my mind :)

@agjohnson
Copy link
Contributor

I have a refactor that i'd like in v0.1.9 of the theme which will allow us to use it as a bower dependency

@agjohnson agjohnson self-assigned this Jul 31, 2015
@gregmuellegger gregmuellegger added the Improvement Minor improvement to code label Aug 3, 2015
@gregmuellegger
Copy link
Contributor Author

@agjohnson Whats the planned ETA for this? Why not integrating as a python dependency?

@agjohnson
Copy link
Contributor

Our two options here:

  • Bring it in as a python dependency and import the global declaration in our bundled javascript for built docs.
  • Treat it as a javascript dependency as part of the bundling process and bundle the theme javascript into our bundles

I still lean towards the latter, as it's more consistent with how we treat the rest of our javascript dependencies. It's one less spurious script include as well, as it would be bundled in, if that's important. I think the only argument for the python package method is that we're already installing the theme on RTD. This should change though, as I can't think of any reason why the theme should exist outside version virtualenvs.

I'm going to try for this week, it's at the top of my list. I had to bump it from my list last week as my focus wasn't on rtd.org.

@agjohnson
Copy link
Contributor

PR for this is #1645

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Improvement Minor improvement to code
Projects
None yet
Development

No branches or pull requests

3 participants