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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
