jonkeane commented on a change in pull request #11109:
URL: https://github.com/apache/arrow/pull/11109#discussion_r704812737
##########
File path: r/tools/nixlibs.R
##########
@@ -423,13 +423,14 @@ cmake_version <- function(cmd = "cmake") {
)
}
-turn_off_thirdparty_features <- function(env_var_list) {
+turn_off_optional_features <- function(env_var_list) {
Review comment:
This is probably just 🚲 🏠 ing, but I wonder if we should name this
`turn_off_all_optional_features()` so it's clear that we intend everything
optional be be disabled, and we're not picking and choosing some (like we do
elsewhere in this script)
##########
File path: r/tools/nixlibs.R
##########
@@ -203,10 +203,6 @@ system_release <- function() {
read_system_release <- function()
utils::head(readLines("/etc/system-release"), 1)
-is_solaris <- function() {
- tolower(Sys.info()[["sysname"]]) %in% "sunos"
-}
-
Review comment:
It is a bit "wasteful" since we only use it in one location, but I
appreciate that `is_solaris()` is clearer what is going on with this function.
Unless the intention is to make this a little obfuscated.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
