You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This PR reduces the (current) run-time of GitHub Actions from the (somewhat unacceptable, to me) about 26 minutes back to the more common 8 or 9 minutes.
The immediate culprit is the stringr package from the tidyverse with its well-known dependency on stringi requiring a full build of the ICU library each and every time. It accounts for a good ten minutes of this. Technically, the error is at CRAN as the new (source) package appears to be lacking a matching binary package (as binaries get installed when available on the Windows platform for which this applies). This 'bad' state has persisted for around two weeks, if not longer.
In a broader context the culprit however is knitr which we only need for vignettes so this PR changes the setup to the one commonly used by (at least some) other projects, namely to a) not build vignettes (so that we can do with knitr, rmarkdown and its tail build- and run-time dependencies) and to b) not test the vignettes (ditto). That is perfectly defensible as out three vignettes to do not provide additional coverage over the existing unit tests.
The actual change then is three-fold. We tell the remotes package to use its default behavior (of not installing Suggests:), and we tell rcmdcheck to pass the 'no vignettes, please' argument to both the build and check steps, and c) we tell R to not look for package in Suggests: during the actual test. All three are easily reversible at any time. Additionally, we set the 'fail' criterion back to ERROR as WARNING can be a little easy to trigger in a large package (and we will know about it from running tests on other platforms during development too).
Thanks. Also added a NEWS entry for the PR itself.
How would you (and @Shelnutt2) feel if I made a second change that has been bugging me for a bit. The actual workflow YAML files is named R-CMD-check.yaml which is half-correct in that it runs that (and a build) but it does not reflect the architecture. So I am thinking to rename it to maybe windows.yaml or maybe win-msys2.yaml.
Thanks. Also added a NEWS entry for the PR itself.
How would you (and @Shelnutt2) feel if I made a second change that has been bugging me for a bit. The actual workflow YAML files is named R-CMD-check.yaml which is half-correct in that it runs that (and a build) but it does not reflect the architecture. So I am thinking to rename it to maybe windows.yaml or maybe win-msys2.yaml.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR reduces the (current) run-time of GitHub Actions from the (somewhat unacceptable, to me) about 26 minutes back to the more common 8 or 9 minutes.
The immediate culprit is the
stringrpackage from the tidyverse with its well-known dependency onstringirequiring a full build of the ICU library each and every time. It accounts for a good ten minutes of this. Technically, the error is at CRAN as the new (source) package appears to be lacking a matching binary package (as binaries get installed when available on the Windows platform for which this applies). This 'bad' state has persisted for around two weeks, if not longer.In a broader context the culprit however is
knitrwhich we only need for vignettes so this PR changes the setup to the one commonly used by (at least some) other projects, namely to a) not build vignettes (so that we can do withknitr,rmarkdownand its tail build- and run-time dependencies) and to b) not test the vignettes (ditto). That is perfectly defensible as out three vignettes to do not provide additional coverage over the existing unit tests.The actual change then is three-fold. We tell the
remotespackage to use its default behavior (of not installingSuggests:), and we tellrcmdcheckto pass the 'no vignettes, please' argument to both the build and check steps, and c) we tell R to not look for package in Suggests: during the actual test. All three are easily reversible at any time. Additionally, we set the 'fail' criterion back to ERROR as WARNING can be a little easy to trigger in a large package (and we will know about it from running tests on other platforms during development too).