MichaelChirico commented on a change in pull request #28365:
URL: https://github.com/apache/spark/pull/28365#discussion_r415680667
##########
File path: R/pkg/R/install.R
##########
@@ -293,7 +289,7 @@ sparkCachePath <- function() {
Sys.getenv("XDG_CACHE_HOME", file.path(Sys.getenv("HOME"), ".cache")),
"spark")
}
} else {
- stop(sprintf("Unknown OS: %s", .Platform$OS.type))
+ stop(gettextf("Unknown OS: %s", .Platform$OS.type, domain = "R-SparkR"),
domain = NA)
Review comment:
`gettextf` is built with a translation interface and thus preferable to
`sprintf`, this is why it's done this way in base R, e.g.
https://github.com/wch/r-source/blob/trunk/src/library/base/R/dataframe.R#L184
```
as.data.frame.default <- function(x, ...)
stop(gettextf("cannot coerce class %s to a data.frame",
sQuote(deparse(class(x))[1L])),
domain = NA)
```
In this way, when `tools::update_pkg_po('.')` is run, the string literal in
`gettextf` is captured as a message for translation; strings for `sprintf` are
not so captured.
`SparkR` doesn't have any translations yet, but IMO it doesn't hurt to align
to that possibility.
----------------------------------------------------------------
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
