Expose renv_version parameter - #13
Merged
Merged
Conversation
shiny2docker() now exposes the renv_version parameter from dockerfiler::dock_from_renv(). Useful for downstream packages that want to opt out of the install_version() / remotes path by passing NULL (install the latest renv from the configured repos). The parameter has no default: when omitted, dock_from_renv() keeps its own default of reading the version from the renv.lock file. An explicit renv_version = NULL is preserved through do.call() via single-bracket list assignment ($<- and [[<- would silently drop the NULL).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR exposes renv_version on shiny2docker(), aligning the wrapper more closely with dockerfiler::dock_from_renv() so callers can control how renv is bootstrapped without bypassing this package.
Changes:
- Added a new
renv_versionargument toshiny2docker()and forwarded it viado.call(), preserving explicitNULL. - Added a regression test covering the
renv_version = NULLpath. - Updated release notes and generated function documentation for the new parameter.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
R/shiny2docker.R |
Adds and forwards the new renv_version argument in the main wrapper. |
tests/testthat/test-shiny2docker.R |
Adds coverage for forwarding renv_version = NULL. |
NEWS.md |
Documents the new parameter in development release notes. |
man/shiny2docker.Rd |
Regenerates the help page to include the new argument. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+31
to
+32
| #' to install the latest available renv from the configured repos | ||
| #' (faster build, no `remotes` dependency). |
|
|
||
| * `shiny2docker()` gains a `renv_version` parameter, forwarded to | ||
| `dockerfiler::dock_from_renv()`. Set to `NULL` to bootstrap with the | ||
| latest available renv (skipping the `remotes` dependency entirely). |
Comment on lines
+117
to
+118
| if (!missing(renv_version)) { | ||
| dock_from_renv_args["renv_version"] <- list(renv_version) |
Address Copilot review feedback on PR #13: - The roxygen and NEWS entries claim that 'renv_version = NULL' skips the 'remotes' dependency entirely. That's only true once the paired upstream change in dockerfiler 0.2.6 is available, so pin the floor in Imports rather than weaken the wording. - Add a regression test for an explicit version string ('renv_version = "1.0.3"') -- the existing forwarding test only exercised the NULL branch, leaving the explicit-string path uncovered at the wrapper layer.
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.
Summary
shiny2docker()is a thin wrapper overdockerfiler::dock_from_renv(). Currently every parameter of the upstream function is forwarded exceptrenv_version— meaning a downstream caller cannot ask "use the latest renv from the configured repos" without bypassingshiny2docker().This PR adds
renv_versionto theshiny2docker()signature and forwards it todock_from_renv().Implementation note: preserving an explicit NULL through do.call()
dock_from_renv()distinguishes three states forrenv_version:NULL-> install the latest renv from the configured repos (skipsremotes);remotes::install_version().We must preserve all three through the wrapper. Naive forwarding fails on
NULL:R's
$<-and[[<-on a list with aNULLvalue delete the entry. The fix is single-bracket assignment withlist()wrapping:This was caught by the new test, which would have failed (
install_versionstill appearing in the Dockerfile, proof thatNULLgot dropped) without the fix.Backward compatibility
renv_versionis added at the end of the signature with no default. Callers that omit it get exactly the same behavior as before (lockfile version is used bydock_from_renv()via its ownmissing()check).Test plan
shiny2docker works— unchanged behavior.shiny2docker forwards renv_version to dock_from_renv— passesrenv_version = NULLand asserts the Dockerfile contains noinstall_versioncall (the invariant that flips when NULL is silently dropped).devtools::test()-> 11 PASS / 0 FAIL.devtools::document()regeneratedman/shiny2docker.Rdconsistently with the new@param.R CMD checkclean (only environmental qpdf warning + clock NOTE on the build sandbox).Pairs with
ThinkR-open/dockerfiler#88for the full benefit (noremotesinstall at all whenNULL), but works on its own against the current releaseddockerfiler.