rstudio / packrat

Packrat is a dependency management system for R
http://rstudio.github.io/packrat/
402 stars 90 forks source link

Update embedded `renv` #662

Closed toph-allen closed 2 years ago

toph-allen commented 2 years ago

This brings the embedded renv code to https://github.com/rstudio/renv/commit/a4c9be3cc0d3e353d043dbad2f069c095e81fb62. This update includes the change that stops inferring quarto dependency for Quarto projects (https://github.com/rstudio/renv/commit/958b084e2dfc8f97d430d40eb12830ad26f49d95).

This PR contains no other changes to Packrat code.

Fixes https://github.com/rstudio/connect/issues/21418

QA Notes

We want to ensure that this change doesn't break dependency detection. One way to exercise this is to deploy a simple Shiny app to an RStudio Connect instance.

After installing the version of Packrat in this PR, deploy the following Shiny app to a Connect instance and ensure that it runs.

library(shiny)

ui <- fluidPage(
    titlePanel("Old Faithful Geyser Data"),
    sidebarLayout(
        sidebarPanel(
            sliderInput("bins",
                        "Number of bins:",
                        min = 1,
                        max = 50,
                        value = 30),
            div("username", verbatimTextOutput("user")),
            div("groups", verbatimTextOutput("groups"))
        ),

        mainPanel(
            plotOutput("plot")
        )
    )
)

server <- function(input, output, session) {
  output$plot <- renderPlot({
    x    <- faithful[, 2]
    bins <- seq(min(x), max(x), length.out = input$bins + 1)

    hist(x, breaks = bins, col = 'darkgray', border = 'white')
  })

  output$user <- renderText(session$user)
  output$groups <- renderText(session$groups)
}

shinyApp(ui = ui, server = server)
aronatkins commented 2 years ago

Should we update NEWS.md to indicate that the quarto package is no longer automatically included?

toph-allen commented 2 years ago

@aronatkins I don't feel strongly either way, and I'm happy to do so if you think it's best. Would you describe it with reference to the bundled renv, or just in simple terms as though it's a change in this package's code?

This also raises the question in my mind: Do we need to update NEWS with all renv discovery ?

aronatkins commented 2 years ago

We already announced the use of renv in a NEWS item. For this change, maybe:

  • Take an renv update to avoid an implicit dependency on the quarto package for all *.qmd content.
toph-allen commented 2 years ago

Updated.