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

Remove RTD Theme workaround #3519

Merged
merged 3 commits into from Jan 20, 2018

Conversation

Projects
None yet
4 participants
@Blendify
Contributor

Blendify commented Jan 15, 2018

This should be merged after rtfd/sphinx_rtd_theme#380 and we use a new release of the theme.

Remove RTD Theme workaround
This should be merged after rtfd/sphinx_rtd_theme#380 and we use a new release of the theme.
@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 15, 2018

This needs to exist for old versions of the theme still.

@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 15, 2018

This could update the comment to mention being for "versions below X" of the theme. We're working on having better data about theme usage, so we can eventually remove it.

Blendify added some commits Jan 15, 2018

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented Jan 15, 2018

This is good to go. The CI failures don't seem to be related.

@davidfischer

This comment has been minimized.

Contributor

davidfischer commented Jan 16, 2018

I'll merge this once rtfd/sphinx_rtd_theme#380 is merged.

@jessetan

This comment has been minimized.

Contributor

jessetan commented Jan 19, 2018

Keeping this workaround should not break anything when the theme is updated (just a duplicate statement), so there is no rush to merge this PR until it is certain old versions of the theme are no longer used (e.g. by authors pinning the theme to a specific version like 0.2.5b2 to benefit from bugs fixed in more recent versions than included by default on RTD)

@ericholscher ericholscher merged commit 23e85d2 into RescueTime-master Jan 20, 2018

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details
@ericholscher

This comment has been minimized.

Member

ericholscher commented Jan 20, 2018

It's just a comment, so I think it's safe to merge now :)

@jessetan

This comment has been minimized.

Contributor

jessetan commented Jan 22, 2018

Oh yeah ;)
I was looking at the original commit which removed the line altogether

@stsewd stsewd deleted the theme-fix branch Aug 15, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment