Change file.info() to file.mtime(), file.size() #4346
Change file.info() to file.mtime(), file.size() #4346klin333 wants to merge 2 commits intorstudio:mainfrom
Conversation
…tra_cols = FALSE)
|
|
gadenbuie
left a comment
gadenbuie
left a comment
There was a problem hiding this comment.
Thank you!
I think we were using file.info() as an optimization, because file.size() and others call file.info() under the hood, but they also nicely incorporate the extra_cols = FALSE flag and after looking into it a little bit I think the performance benefit is negligible.
| # because native.enc is also normally UTF-8 based on *nix) | ||
| if (!isWindows()) return('UTF-8') | ||
| size <- file.info(file)[, 'size'] | ||
| size <- file.info(file, extra_cols = FALSE)[, 'size'] |
There was a problem hiding this comment.
| size <- file.info(file, extra_cols = FALSE)[, 'size'] | |
| size <- file.size(file) |
|
arghhh... i committed using my employment email address, and when i signed the CLA, github auto put my employment email address wrong, and now i can't re-sign the CLA... Is there something that maintainers can do to allow me to re-sign, and i'll put in the correct email address matching my commits? really really sorry about this... |
|
@klin333 thanks but don't worry about the CLA! We've switch Shiny to an MIT license and the CLA is less important now; we'll probably disable the CLA bot soon. And (not to diminish your contribution in any way), this PR isn't likely copyrightable anyway. |
|
thanks @gadenbuie. any chance you can help allow this merge through. it seems to be currently stuck at the licensing cla check. |
| list.files(dir, pattern = filePattern, recursive = TRUE, ignore.case = TRUE) | ||
| ) | ||
| times <- file.info(files)$mtime | ||
| times <- file.mtime(files) |
There was a problem hiding this comment.
file.mtime() was added in R 3.2.0 -- should we add a Depends constraint to DESCRIPTION @gadenbuie?
There was a problem hiding this comment.
@gadenbuie more generally, would you like to get this one over the finish line (i.e., NEWS.md file, address suggestions, etc)?
5 participants
By default, file.info() has extra_cols = TRUE. This gets very annoying on Windows since R4.5.0, which added extra columns which sometimes can fail and give warnings.
All uses of file.info in shiny package does not need these extra columns anyway. So this PR changes file.info() to the R provided convenience functions of file.mtime(), file.size() and file.info(..., extra_cols = FALSE)