golem icon indicating copy to clipboard operation
golem copied to clipboard

Enhance sanity_check

Open moodymudskipper opened this issue 4 years ago • 8 comments

As discussed on twitter : I propose to integrate the features from {shinycheck} into {golem}'s sanity_check().

I've worked on it already and could make half of the features work nicely already, it is really handy with rstudioapi::sourceMarkers! Everything needs to be coded differently though to track line and column positions of problematic code, that's what take time.

I propose to finish implementing my ideas, then get your general feedback, then maybe brainstorm new features (if you already have ideas please shoot). Formal testing might also be a challenge.

Does it sound right ?

moodymudskipper avatar Dec 30 '20 14:12 moodymudskipper

See pull request for first iteration.

To go further I'd love to :

  • Have a bit of general feedback
  • Know if I can call devtools::load_all() from the function (the scripts use some match.call() to check arguments and will work optimally only if the functions that exist are found)

moodymudskipper avatar Jan 02 '21 20:01 moodymudskipper

@ColinFay I wanted to give you a peek at the first step but it was too buggy, I'll show you when I have something more solid, meanwhile I have a couple questions :

  • Is it fair to expect users to define ONLY mod_<module_name>_ui and mod_<module_name>_server in mod_<module_name>.R ? Or is it fine for them to define R functions there ? I can either warn or ignore if I find some.
  • Do you think you will switch at some points to the new RStudio conventions that Hadley pushes in his book, namely using moduleServer instead of callModule and/or wrapping a call to it on your module server function, as in the example below :
histogramServer <- function(id) {
  moduleServer(id, function(input, output, session) {
    data <- reactive(mtcars[[input$var]])
    output$hist <- renderPlot({
      hist(data(), breaks = input$bins, main = input$var)
    }, res = 96)
  })
}

moodymudskipper avatar Jan 04 '21 12:01 moodymudskipper

Hey @moodymudskipper,

Sorry for the long delay with this...

Do you think this is something you'd still want to work on?

If yes, I'm thinking about including this feature on the new release of {golem} (expected date : 2021-11-07).

ColinFay avatar Oct 01 '21 09:10 ColinFay

Hi Colin,

I haven't worked on this in a while and I think it was a bit complex. I'm not sure I'll have enough time in a month, but I can try with no promise!

moodymudskipper avatar Oct 01 '21 09:10 moodymudskipper

No rush :)

Let me know !

For your two other q :

  • Yes, it's fair to assume that there will only be the two modules inside a mod_ file. Functions are to be put inside an fct_ or utils_ file.
  • We have the new skeleton since v 0.3.1, so that's the default now :)

Thanks !

ColinFay avatar Oct 01 '21 10:10 ColinFay