[EXPERIMENT — do not merge] test whether dependencies: "hard" is still needed - #37
Closed
eliotmcintire wants to merge 12 commits into
Closed
eliotmcintire wants to merge 12 commits into
eliotmcintire wants to merge 12 commits into
Conversation
All changes have been used in FireSenseTesting for some time; time to merge
Firesense_LCC_flammability downloads from ftp.maps.canada.ca (LCC) and cwfis.cfs.nrcan.gc.ca (fire polygons) at build time. R-CMD-check runners routinely can't reach these, failing the vignette build with "Failed to connect to ... port 443". Probe both servers up front and skip the live download/analysis chunks when either is unreachable, degrading to documentation-only (same as the existing macOS/archive path). Adds curl to Suggests for the reachability probe. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Replaces this repo's hand-rolled matrix with a thin caller, so third-party action versions, system dependencies, caching and the check itself are maintained in one place instead of 17. Bumping actions/checkout there now updates every repo at once. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012DVjmY3im9Xak7tXLSMGCa
The org concurrency budget is shared across every PredictiveEcology repo and is the binding constraint on getting work through. These legs added R versions below the default matrix; testing older R is explicitly not worth the runner contention it causes. oldrel-N is relative, so the default matrix's floor still rises on its own with each R release -- no standing commitment to any particular old version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012DVjmY3im9Xak7tXLSMGCa
This reverts commit 654d7ab.
NLMR is not referenced anywhere in this package -- not in DESCRIPTION, R/,
vignettes/, tests/ or man/. Its only appearances were the two CI workflows,
which installed it from GitHub on every leg:
NLMR=?ignore
post-install: pak::pkg_install("ropensci/NLMR")
That is the same dead weight the PE CI standards call out from SpaDES, where
ropensci/NLMR and s-u/fastshp were installed on every pkgdown build while
being referenced nowhere -- the slowest step and the largest failure surface.
Removed from both R-CMD-check.yaml and test-coverage.yaml (this PR migrates
only the former, but the install was equally unused in the latter, and "remove
it entirely" means both).
Also sync extra-packages to the current Suggests:
- drop cowplot: promoted to Imports in 7b5afe5, so it now installs as a hard
dependency and does not belong in extra-packages.
- add curl: a real Suggests, and the LCC vignette's reachability probe is
gated on requireNamespace("curl") -- without it the probe silently degrades
to "skip" rather than testing anything.
Both files parse as valid YAML.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
…l live Experiment, not for merge. Branched from ci/migrate-to-org-workflow so the only difference is the dependency resolution mode. The old workflow set `dependencies: '"hard"'` to work around a pak solver bug: the combined Depends+Imports+Suggests graph tripped over an inconsistent Matrix entry (R>=4.7 from /src/contrib/4.7.0/Recommended coexisting with R>=4.4 from the main /src/contrib path), and the conflict only materialised when Suggests was traversed transitively. The 16 hand-listed Suggests in extra-packages exist solely to re-add what "hard" excludes. That failure mode depends on CRAN's metadata layout at a point in time, so it may have aged out. This run tests it: `dependencies: '"all"'` (the org default) with the extra-packages block removed entirely -- the 16 entries matched DESCRIPTION's Suggests exactly, so "all" should install the same set straight from DESCRIPTION. Green => "hard" and the 16 lines can be deleted from #31, and the DESCRIPTION/workflow drift that required manual syncing goes away. Red with a Matrix/solver error => the workaround is still load-bearing and should be kept, with the comment restored. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
eliotmcintire
added a commit
that referenced
this pull request
Sep 1, 2026
The `dependencies: '"hard"'` setting plus 16 hand-listed Suggests worked around a pak solver bug: an inconsistent Matrix entry (R>=4.7 from /src/contrib/4.7.0/Recommended coexisting with R>=4.4 from the main /src/contrib path) that only surfaced when Suggests was traversed transitively. Hard-deps-only resolved, and the Suggests were re-added by hand. That failure depended on CRAN's metadata layout at a point in time. Retested in #37 (branched from this one, so the only variable was resolution mode): `dependencies: '"all"'` with no extra-packages list came back 12/12 green -- every platform, every R version including devel, and the nosuggests leg. The metadata that caused it has moved on. - R-CMD-check.yaml: '"hard"' -> '"all"', extra-packages list removed (16 entries, which duplicated DESCRIPTION's Suggests exactly). any::rcmdcheck is supplied by the reusable workflow. - test-coverage.yaml: same change. Its comment deferred to R-CMD-check.yaml for a rationale this migration would have deleted, and its mirrored-Suggests note no longer applied. This removes the drift class that bit earlier today: the list had gone stale against DESCRIPTION (cowplot promoted to Imports in 7b5afe5 and still listed; curl a Suggests and missing, which silently degraded the LCC vignette's reachability probe to "skip"). There is now nothing to keep in sync. The workaround is documented in-place, so if a pak resolve error naming Matrix ever returns, the fix is recorded rather than rediscovered. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon
Contributor
Author
|
Answered: the workaround has aged out. 12/12 green — every platform, every R version including devel, plus the The pak Closing — this branch existed only to run the experiment. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Draft / experiment. Not for merge. Opened only to get CI to run — the workflow triggers on
pull_request, so a branch alone gets nothing.Question
Is the pak solver bug that forced
dependencies: '"hard"'still live?The old workflow documented it:
That failure depends on CRAN's metadata layout at a moment in time, so it is plausibly transient.
What this branch changes
Branched from
ci/migrate-to-org-workflow(#31) so the only difference is dependency resolution:The 16
extra-packagesentries matchedDESCRIPTION's Suggests exactly (I synced them in #31), so"all"should install the same set directly fromDESCRIPTION.any::rcmdcheckis added by the org workflow itself, andextra-repositoriesalready defaults to the PredictiveEcology r-universe — whichscfmutilsneeds, since it is not on CRAN.How to read the result
dependencies: '"hard"'and the 16 lines from ci: migrate R-CMD-check to the org reusable workflow #31.DESCRIPTIONbecomes the single source of truth, and the workflow/Suggests drift that needed hand-syncing this week goes away permanently.Matrix→ still load-bearing. Keep it in ci: migrate R-CMD-check to the org reusable workflow #31, and add the explanatory comment that the migration dropped, so nobody deletes it later on the reasonable-looking grounds that the list is redundant.Either way this branch gets closed, not merged.
Refs #31
🤖 Generated with Claude Code
https://claude.ai/code/session_01T3NTQTNStSLZ3mA3CsLcon