-
Notifications
You must be signed in to change notification settings - Fork 186
Implement all_linters()
wrapper to access all available linters
#1854
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
Conversation
Can we add integration with |
Codecov Report
@@ Coverage Diff @@
## main #1854 +/- ##
=======================================
Coverage 98.86% 98.86%
=======================================
Files 112 112
Lines 4838 4839 +1
=======================================
+ Hits 4783 4784 +1
Misses 55 55
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
We need much more thorough cross referencing and update our examples that use A new user should be able to easily discover all three variants and understand how and when to use what. |
Good idea. I have
|
#' - [linters] for a complete list of linters available in lintr. | ||
#' @export | ||
all_linters <- function(packages = "lintr", ...) { | ||
linters_with_tags(tags = NULL, packages = packages, ...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happens in the unlikely case that tags
is given as a named argument?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will produce an error:
library(lintr)
all_linters(tags = "default")
#> Error in linters_with_tags(tags = NULL, packages = packages, ...): formal argument "tags" matched by multiple actual arguments
Created on 2022-12-18 with reprex v2.0.2
But I doubt anyone will do this because this assumes that the user has looked at the implementation details for this newly introduced function and figured out that all_linters()
is a wrapper around linters_with_tags()
, and is deliberately using this argument. But, in that case, they shouldn't be surprised by the error message they get.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
modify_defaults(..., defaults = linters_with_tags(tags = NULL, packages = packages)
should not suffer from this bug.
vignettes/lintr.Rmd
Outdated
```r | ||
linters: linters_with_defaults( | ||
defaults = linters_with_tags(tags = NULL) | ||
defaults = all_linters() | ||
) | ||
``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should also directly call all_linters()
.
Is there a section mentioning linters_with_tags()
?
I will fix the vignette later today. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, thanks!
vignettes/lintr.Rmd
Outdated
|
||
You can include tag-based linters in the configuration file, and customize them further: | ||
|
||
```r |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed, these config examples should be ``` yaml
for consistency with ll. 77 and clearer separation of R code examples and config examples.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea. Done.
Closes #1843