[GitHub] [spark] MichaelChirico commented on a change in pull request #28419: [MINOR][R] small tidying of sh scripts for R

[GitBox <gi...@apache.org>]

MichaelChirico commented on a change in pull request #28419:
URL: https://github.com/apache/spark/pull/28419#discussion_r418009756



##########
File path: R/create-rd.sh
##########
@@ -34,4 +34,4 @@ pushd "$FWDIR" > /dev/null
 . "$FWDIR/find-r.sh"
 
 # Generate Rd files if devtools is installed
-"$R_SCRIPT_PATH/Rscript" -e ' if("devtools" %in% rownames(installed.packages())) { library(devtools); setwd("'$FWDIR'"); devtools::document(pkg="./pkg", roclets=c("rd")) }'
+"$R_SCRIPT_PATH/Rscript" -e ' if(requireNamespace("devtools", quietly=TRUE)) { setwd("'$FWDIR'"); devtools::document(pkg="./pkg", roclets="rd") }'

Review comment:
       `requireNamespace` is a more minimal test to be sure `devtools::` will work correctly.
   
   This is the more common route to check for available functionality, e.g. for Suggested (not required) packages. this is how the suggested `arrow` dependency is handled, for example:
   
   ```
   grep -r "requireNamespace" R/pkg/R -n
   R/pkg/R/types.R:91:  if (!requireNamespace("arrow", quietly = TRUE)) {
   R/pkg/R/deserialize.R:235:  if (requireNamespace("arrow", quietly = TRUE)) {
   R/pkg/R/SQLContext.R:152:  if (requireNamespace("arrow", quietly = TRUE)) {
   R/pkg/R/serialize.R:225:  if (requireNamespace("arrow", quietly = TRUE)) {
   R/pkg/R/DataFrame.R:1229:              if (requireNamespace("arrow", quietly = TRUE)) {
   ```
   
   The `installed.packages` is much more cumbersome -- it looks up the full set of available packages, then searches to see if this particular one is there.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org