hadley / r-pkgs

Building R packages
https://r-pkgs.org
Other
880 stars 630 forks source link

Feedback on Chapter 2: The whole game #799

Closed jthomasmock closed 2 years ago

jthomasmock commented 2 years ago

Generally, is it worth namespacing all of the functions so that people know where they're coming from in this chapter? This is not as big of an issue since it's really devtools, usethis, testthat, as opposed to tidymodels or tidyverse where there are many packages. However, I know that working with newer learners they can get a bit confused when trying to do small components and may not have all the necessary packages loaded.

Section 2.4:

Section 2.8:

  1. Note that load_all() has made the strsplit1() function available, although it does not exist in the global environment.

    [exists](https://rdrr.io/r/base/exists.html)("strsplit1", where = [globalenv](https://rdrr.io/r/base/environment.html)(), inherits = FALSE)
    #> [1] FALSE

2. In the "RStudio exposes load_all() callout, is it with adding when inside a R package? This is different when in RMarkdown/Quarto/blog/etc.

Section 2.10:

There is a great RStudio callout on ctrl + . - I checked and this is valid on all platforms. Not sure where it would be best to "live" in the book, but the command palette (ie Cmd + Shift + P) can also execute devtools and other RStudio package related commands. Might be worth having an appendix linking out to RStudio/package cheatsheets, RStudio command palette and shortcut documentation. Repeated similar suggestions below. Command palette hits all of the RStudio helpers in build pane and more!

Screen Shot 2022-06-24 at 9 39 45 AM

Section 2.12

In the "RStudio exposes document()` callout, is it with adding when inside a R package? This is different when in RMarkdown/Quarto/blog/etc. Also same feedback, this is possible with Command Palette > "Document Package"

Section 2.14

In the "RStudio exposes install callout, is it with adding when inside a R package? This is different when in RMarkdown/Quarto/blog/etc. Also same feedback, this is possible with Command Palette > "Install Package"

Section 2.15

In the "RStudio exposes testthat callout, is it with adding when inside a R package? This is different when in RMarkdown/Quarto/blog/etc. Also same feedback, this is possible with Command Palette > "Test"

jennybc commented 2 years ago

Generally, is it worth namespacing all of the functions so that people know where they're coming from in this chapter?

My intention is to only call functions in devtools (or usethis, but devtools Depends on usethis and therefore attaches it). So I think we're OK.

I updated language re: how to toggle visibility of hidden files in RStudio for consistency with other terminology and to mention the gear symbol.

I am not familiar with hello.R. Maybe this memory is from some earlier point I history or from a non-devtools/usethis workflow?

Worth mentioning that RStudio will also throw a warning if you have defined a global function and then devtools::load_all()

FYI this is pkgload throwing the warning, not RStudio. I'm OK with the warning standing on its own, i.e. I don't think we need to warn people that they might see the warning.

Likewise I think it's OK to omit the "when inside an R package" from these RStudio callouts, because it feels almost implied from context (the book, the chapter, etc.)