-
-
Notifications
You must be signed in to change notification settings - Fork 86
New pkgdown theme #378
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
New pkgdown theme #378
Conversation
|
Just started looking at this. First things I noticed looking at the GHA logs are that the new Update: test failures are definitely unrelated to this. I'm fixing that on a separate branch: #379 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #378 +/- ##
=======================================
Coverage 98.62% 98.62%
=======================================
Files 35 35
Lines 5750 5750
=======================================
Hits 5671 5671
Misses 79 79 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
So tests seem to be ok, but I can't see the new pkgdown action, so we need to check that before merging. |
Yeah tests should be fixed by the branch that I merged in. But I also don't see the pkgdown action. |
|
It seems like the action must be run on PRs before it appears--otherwise it can only be seen on main |
|
https://mc-stan.org/bayesplot/dev/ It's up now! Looks like some plots on the main page are broken so I'll work on that. Plots on other pages, viz. a vignette I chcked, seem to be fine. |
|
Fixed without having to move things. Should be ready for review. |
|
Looking good! Just a couple of things I notice in the dev site:
|
Do we need to manually run the workflow to regenerate the dev site? |
|
For now yes--disabled runs on PRs. |
|
The badge and plots seem to be fixed. |
|
For some reason the same ggplot2 theme issue is happening on the function documentation pages in the Examples sections. E.g., compare the plots at the bottom of these pages:
I guess we can manually set the theme in each examples section, but I don't understand why this needs to be done when it didn't need to be previously. I think it must have to do with some change in ggplot2 v4.0 that's preventing bayesplot's default them from being set by default, that's the only thing I can think of. |
I guess that can be fixed (if possible) separately and we can go ahead with the new site since it's (probably) not an issue with the site itself. Let me think about it for a min though. |
|
Sorry reread your first comment. I'm curious if the same thing happens rendering the old site but bumping the ggplot dependency to 4.0.0 |
|
I think the same thing would happen. I'm 99% sure it's a ggplot v4.0 thing. I'm working on fixing it. I sort of have a fix, but there are some subtleties I'm still working out. |
|
I just merged in a fix (I think) for the ggplot theme issue |
Updating the theme to better match the main Stan page. This PR uses the new theme & use a GitHub Action to automatically build the dev version of the site on PRs and the release version on releases.
The action needs to be tested to see if all dependencies are properly specified.