Cleanup installs happening in the github workflows #409
Conversation
Since pre-commit isn't part of the team workflow, don't clutter the repo with any related config files
bsweger
force-pushed
the
bsweger/cleanup-workflow-and-dependencies
branch
from
33ce2b8 to
ba20046
Compare
bsweger
changed the title
bsweger
force-pushed
the
bsweger/cleanup-workflow-and-dependencies
branch
5 times, most recently
from
0298443 to
5c3d1b3
Compare
|
@bsweger I'm assuming you will ping me when this is ready for a full review, and am holding off on a review until then. |
.github/workflows/pkgdown.yaml
Outdated
| devtools::install_github("epiforecasts/scoringutils") | ||
| remotes::install_deps(dependencies = NA) | ||
| install.packages(c("devtools","pkgdown")) | ||
| devtools::install_github("eepiforecasts/[email protected]") |
There was a problem hiding this comment.
I assumed the scoringutils version here should match the changes made in #400
bsweger
changed the title
|
@nickreich ready for review! |
@elray1 is I recall that github actions have some capacity for ad-hoc runs, but I haven't looked into it yet. |
There was a problem hiding this comment.
It feels like I'm breaking some GitHub code of conduct to give an approving review to a PR that is not passing a check, but I did get a few minutes to look at this and it the changes look good to me. So once the checks are passing I don't have other concerns.
bsweger
force-pushed
the
bsweger/cleanup-workflow-and-dependencies
branch
2 times, most recently
from
40efe2b to
6e69a9c
Compare
The vignette should use the scoringutils package made available via covidHubUtils, rather than trying to install it again (which can cause an "unload" error). First pass at removing some the explicit dependency install in the R CHECK workflow Also removed tidyverse dependency in align.R as part of this cleanup. Remove RSocrata install I don't *think* we're downloading anything from Socrata in this package remove here and textshaping installs from the github workflows Remove the extraneous "remotes" install in the github workflows We still have #406 to get the covidHubUtils workflows in better alignment with those in the hubverse, but for now we're focused on cleaning up workflow installs per # 404 by either moving them to DESCRIPTION (if they're used) or deleting them (if they're not). Fixup to reinstate the specific version for scoringutils Move covidData to a first-class dependency (rather than 'suggests') Removing some of the explicit dependency installs from github workflows broke things, because covidData wasn't being installed with the command install_deps(NA). Since covidData is used by covidHubUtils, let's make sure it's always installed and try to avoid ad-hoc package installation in CI. Fix vignette errors by making sure the packages in "suggests" are installed Remove explicit installs of zoltr and covidData Let's see what happens when we remove the build steps that install zoltr and covidData (since these two packages are already listed as dependencies in covidHubUtils) fixup PR feedback Try making sure the package itself is installed Streamline dependency installs in pkgdown.yaml
bsweger
force-pushed
the
bsweger/cleanup-workflow-and-dependencies
branch
from
1d55cb1 to
878367f
Compare
|
@elray1 I believe I found and fixed the issue that was causing the tests to fail (that said, I'm not sure why they were passing before merging in your requested changes (which seem unrelated..and I reverted them to just see if the tests would start passing again, and they did! :tableflip:) Anyway, everything is green now, and I squashed the commits so the history is a bit cleaner. Can you re-review when you have a chance? Thanks! |
resolves #404, resolves #405
The scope of these commits is making sure that all required dependencies are in the package's DESCRIPTION instead of being explicitly installed as a workflow step (for example, DT is required to build the vignettes).
Additionally, there's some cleanup to remove workflow-based installs if they aren't being used and/or reducing the footprint (e.g., explicitly use
ggplot2in the vignette rather than pull in the entire tidyverse).Longer term, we have a goal to make these workflows better align to the newer ones used in the hubverse (#406 )
note: there are several smaller commits so it's easier to revert any breaking changes...happy to squash after approval to tidy up the commit history