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

Revert PR#123 changes in favor of pydata-sphinx-theme v0.7.0 #160

Merged
merged 3 commits into from
Nov 15, 2021

Conversation

erogluorhan
Copy link
Member

Closes #128 by reverting the changes of PR #123.

pydata-sphinx-theme released v0.7.0 that fixes their Google Analytics handling.

@github-actions
Copy link

github-actions bot commented Oct 18, 2021

This pull request is being automatically built with GitHub Actions and Netlify. To see the status of your deployment, click below.

🔍 Git commit SHA: e734b4f
✅ Deployment Preview URL: https://6192e2bd5b2c1600c50cb148--pythia-foundations.netlify.app

@erogluorhan
Copy link
Member Author

erogluorhan commented Oct 18, 2021

@kmpaul @brian-rose ,

There is an issue here, which I could not figure out yet and will not have the availability for a while to further work on it:

  1. I reverted all of the changes we had made in PR Google Analytics Workaround Until pydata_sphinx_theme Fix Released #123 , but in that case we are still having the erroneous v0.6.3 of pydata-sphinx-theme being installed in local and remote (Netlify) tests. I think it is due to our extension chain of jupyter-book -> sphinx-book-theme -> pydata-sphinx-theme because we don't explicitly have pydata-sphinx-theme dependency listed in our environment.
  2. I attempted adding pydata-sphinx-theme>0.6.3 into our environment, but the tests then started failing at some jupyter-book initialization steps I think.

So, it looks like jupyter-book might not yet be compatible with v0.7.1 of pydata-sphinx-theme.

You are welcome to look into this issue; otherwise, I will need to close this until I have time to look into it again (This PR is not urgent as our Google Analytics config is currently working fine)

@brian-rose
Copy link
Member

Yes, it seems that the conda-forge recipe for sphinx-book-theme is currently pinned to

- pydata-sphinx-theme >=0.6.0,<0.7re

which you can find here:
https://github.com/conda-forge/sphinx-book-theme-feedstock/blob/50cf937fc17423cecf99f9fb63a670a46082950c/recipe/meta.yaml#L30

@brian-rose
Copy link
Member

So maybe we just need to let this simmer until things get updated upstream.

@erogluorhan
Copy link
Member Author

Yes, it seems that the conda-forge recipe for sphinx-book-theme is currently pinned to

- pydata-sphinx-theme >=0.6.0,<0.7re

which you can find here: https://github.com/conda-forge/sphinx-book-theme-feedstock/blob/50cf937fc17423cecf99f9fb63a670a46082950c/recipe/meta.yaml#L30

Oh, thanks for this, I didn't look at it.

@kmpaul kmpaul added the infrastructure Infrastructure related issue label Oct 21, 2021
@erogluorhan
Copy link
Member Author

erogluorhan commented Nov 1, 2021

Opened Issue#423 at sphinx-book-theme for this.

@andersy005
Copy link
Member

Now that there's a new version of sphinx-book-theme, it might a good time to revisit this

@erogluorhan
Copy link
Member Author

Now that there's a new version of sphinx-book-theme, it might a good time to revisit this

Yes, I will, thanks!

@kmpaul
Copy link
Contributor

kmpaul commented Nov 12, 2021

Yes. With the newest release, the Jupyter Book config option for GA should work, even with newer G-??? tags.

@erogluorhan erogluorhan marked this pull request as ready for review November 15, 2021 22:25
@erogluorhan erogluorhan requested review from a team as code owners November 15, 2021 22:25
@erogluorhan erogluorhan requested review from ktyle, clyne and dcamron and removed request for a team November 15, 2021 22:25
Copy link
Contributor

@kmpaul kmpaul left a comment

Choose a reason for hiding this comment

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

Great! Thanks, @erogluorhan!

@erogluorhan erogluorhan merged commit 663db98 into main Nov 15, 2021
@erogluorhan erogluorhan deleted the revert_123 branch November 15, 2021 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Infrastructure related issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Revert Google Analytics Workaround Once pydata_sphinx_theme Fix Released
4 participants