Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix cache logic #146

Merged
merged 6 commits into from
May 18, 2016
Merged

fix cache logic #146

merged 6 commits into from
May 18, 2016

Conversation

schloerke
Copy link
Contributor

@schloerke schloerke commented May 6, 2016 •
edited
Loading

Hi Jim,

when turning on the cache flag, I wasn't getting any speed improvements. With this pull request, files are being saved and compute much faster.

(on the GGally package...)

load_all("../../../lintr/lintr/"); 
# cache empty
system.time({lint_package(cache = TRUE)})
# Loading lintr
# .............................................
#    user  system elapsed 
#116.968   1.501 121.804 

# cache filled
system.time({lint_package(cache = TRUE)})
# .............................................
#    user  system elapsed 
#  11.891   0.275  12.743

Best,
Barret

@schloerke
separate cacheDir and cache boolean logic
ba9e590
@schloerke
force the evaluation of exclusions and pass them into lint
71fb294 
lazy evaluation causes these to be evaluated after reading the settings
when cache is TRUE.  By this time, settings$exclusions is back to the
original non-normalized paths
@schloerke
missed variable change
e8799fb
@schloerke
allows for cache to be a string when supplied or if not equal to true
b599a26 
passes tests now
@schloerke
Copy link
Contributor Author

updated to pass R CMD check and travis

jimhester
jimhester reviewed May 18, 2016
View reviewed changes
R/lint.R
if (length(cache)) {
lint_cache <- load_cache(filename, cache)
if (length(cacheDir)) {
lint_cache <- load_cache(filename, cacheDir)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use follow the naming convention and use cache_dir here

@jimhester
Copy link
Member

Please add a note to NEWS.md describing the fix and thanking yourself, then ping me.

Thank you for the PR!

@schloerke
Copy link
Contributor Author

ping

No problem! I really appreciate both lintr and covr. Both are wonderful packages to have.

@jimhester jimhester merged commit 104f2d9 into r-lib:master May 18, 2016
@jimhester
Copy link
Member

Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@schloerke @jimhester