jonkeane commented on a change in pull request #11000:
URL: https://github.com/apache/arrow/pull/11000#discussion_r695904613
##########
File path: r/tools/nixlibs.R
##########
@@ -215,10 +215,8 @@ download_source <- function() {
# Given VERSION as x.y.z.p
p <- package_version(VERSION)[1, 4]
- if (is.na(p) || p < 1000) {
- # This is either just x.y.z or it has a small (R-only) patch version
- # Download from the official Apache release, dropping the p
- VERSION <- as.character(package_version(VERSION)[1, -4])
+ if (is.na(p)) {
Review comment:
Do we want to remove the p < 1000 checking here? I think we want to keep
that (or maybe check < 9000 since we don't want dev versions to be attempted to
be downloaded, right?
##########
File path: r/tools/nixlibs.R
##########
@@ -215,10 +215,8 @@ download_source <- function() {
# Given VERSION as x.y.z.p
p <- package_version(VERSION)[1, 4]
- if (is.na(p) || p < 1000) {
- # This is either just x.y.z or it has a small (R-only) patch version
- # Download from the official Apache release, dropping the p
- VERSION <- as.character(package_version(VERSION)[1, -4])
+ if (is.na(p)) {
+ # This is just x.y.z so download the official Apache release
Review comment:
This is probably not the time to bikeshed, but I wonder if a try/catch
(or "just" a modified if/else like below) would provide future proofing for the
all-R no-cpp release that the removed comment hinted at.
(I couldn't get GH's UI to let me edit all of these lines, but this would
replace 219-224:
```
# This is just x.y.z so download the official Apache release
got_source <- apache_download(VERSION, tf1)
# If this source isn't available, fall back to x.y.z (in case this is an
R-only patch version that doesn't have a cpp patch
if (!got_source) {
VERSION <- as.character(package_version(VERSION)[1, -4])
got_source <- apache_download(VERSION, tf1)
}
if (got_source) {
untar(tf1, exdir = src_dir)
unlink(tf1)
src_dir <- paste0(src_dir, "/apache-arrow-", VERSION, "/cpp")
}
```
--
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]
