-
Notifications
You must be signed in to change notification settings - Fork 186
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.
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
handle nested terminal if/else in return_linter() #2362
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2362 +/- ##
==========================================
- Coverage 99.11% 99.09% -0.02%
==========================================
Files 125 125
Lines 5650 5658 +8
==========================================
+ Hits 5600 5607 +7
- Misses 50 51 +1 ☔ View full report in Codecov by Sentry. |
I was also a bit worried about performance -- would overhead to recursing in R push us to pursue this as pure XPath? I am satisfied with the R-recursion approach -- timing is not meaningfully different (faster, if anything) & should be much more readable. I linted all base R packages as a stress test: setwd("src/library")
f = list.files()
f = f[file.exists(file.path(f, "DESCRIPTION"))]
lint_all = \(pkgs, linters) for (pkg in pkgs) lint_package(pkg, linters = linters)
# 'old' = current main, 'new' = current this PR
system.time(lint_all(f, old_explicit))
# user system elapsed
# 72.638 1.162 74.112
system.time(lint_all(f, old_implicit))
# user system elapsed
# 66.546 0.819 67.682
system.time(lint_all(f, new_explicit))
# user system elapsed
# 72.178 1.058 73.604
system.time(lint_all(f, new_implicit))
# user system elapsed
# 62.563 0.871 63.674 |
Closes #2354, Closes #2356
NEWS is missing, but I'll put it into the expanded item for #2321 (depending on which is merged first)