-
-
Notifications
You must be signed in to change notification settings - Fork 37
Reformat pkgdown workflow #292
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
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #292 +/- ##
=======================================
Coverage 92.76% 92.76%
=======================================
Files 31 31
Lines 2986 2986
=======================================
Hits 2770 2770
Misses 216 216 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
On second though, if possible, it might make more sense to automatically build and deploy sites to https://dev.mc-stan.org/loo/ and setup another workflow to generate the site on releases which would get pushed to https://mc-stan.org/loo/. |
|
Obviated |
Yeah I agree that it would be preferable to only publish the site on https://mc-stan.org/loo/ when we do a release. If we publish at the main URL on every commit to master then the website could advertise unreleased features that aren't available on CRAN yet. |
|
I think this looks good. But the regenerated website pages that are part of this PR are for the development version of loo, not the released version. So if I merge this won't it update the website to the development version? |
|
Right, I'll move files from the gh-pages branch into the docs folder then |
|
https://github.com/stan-dev/loo/actions/runs/15627201098/job/44023603976 Looks like the vignettes get built too. |
|
Updated |
Comment out push trigger for main and master branches
Pulled in fix for NAs which was breaking vignettes
|
@jgabry this should build the site in the pages directory and doesn't add any website files to the main branch. I'm running the pkgdown action again but hopefully it passes and this can be merged. |
|
Cool thanks, I think this looks great. I put two questions in review comments |
|
I think this is ready to go except for updating the config after transferring the repo. |
Merge Aki's documentation updates
_pkgdown.yml
Outdated
| url: https://mc-stan.org/loo/ | ||
| destination: "docs/" | ||
| template: | ||
| package: pkgdownConfig |
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.
Does this also need to be pkgdown-config now?
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.
So the package itself has to be pkgdownConfig since that's the R package's name--but the repository now needs to be stan-dev/pkgdown-config
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.
The builds are starting to fail now that its trying to point to the stan-dev/pkgdown-config repository somehow--there's some dependency issue. The cache might be stale and so needs to be refreshed?
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.
Looks like it's passing now
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.
The website still hasn't been rebuilt--the new "DEV" tag isn't visible. There seems to be some dependency issue but I don't understand why there should be any?
|
Yeah I'm not sure why it's not updating. Unrelated, but in the vignette do we also need to change |
|
Yeah so this is failing due to dependency issues but it doesn't make any sense to me https://github.com/stan-dev/loo/actions/runs/18600052282/job/53036048076 |
|
What about resetting the cache? Although it looks like you're not even using the cache action, it's commented out |
|
I'm also baffled about the dependencies. I did delete the cache but only for this branch--I guess it could still be hitting the main branch's cache? I'm really confused about what it is complaining about and why it started now. You're right about the vignette as well that needs to be the package name. Just double checked and it doesn't seem like there were any cache hits in the run logs |
|
Long shot but there's usually an "Explain error" (or something like that) button that opens a chat with GitHub Co-pilot to try to help you troubleshoot the failure. I've tried it before in other repos and it usually wasn't helpful but one time it did actually help me sort out an error. |
|
Yeah I had tried that and it said nothing useful. I'm looking further into this of course--hopefully it's just a simple mistake I made somewhere. |
|
@jgabry Fixed it! For posterity: removed the pkgdown-config repo from Config/needs/website and added it manually as a dependency in the GHA. Maybe hyphens somehow were the issue? |
|
Awesome, thanks for tracking down the issue! One question: if for any of the other R packages we don't end up using GHA to generate the website then will how will it resolve the dependency issue of requiring the pkgdown config package from stan-dev/pkgdown-config? That is, suppose for rstanarm we decide we'll do it manually using How will it know where to find this? Do that make sense? I might just be missing something obvious. |
|
I included a little section here: https://mc-stan.org/pkgdown-config/articles/Setup.html You don't have to use For local development, you need to install the package before you can build the site: pak::pak("stan-dev/pkgdown-config")
pkgdown::build_site()The workflow without the action would then be: checkout gh-pages branch, install pkgdownconfig, build the site, and push manually. |
|
Ah ok, perfect, thanks. |
jgabry
left a comment
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.
Thanks for all your work on this @VisruthSK. Do you want to do the honors and merge it?
mc-stan.org/loo/dev/*