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.

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

New which_grepl_linter #2281

Merged
merged 11 commits into from
Nov 18, 2023
Merged

New which_grepl_linter #2281

merged 11 commits into from
Nov 18, 2023

Conversation

MichaelChirico
Copy link
Collaborator

Part of #884

No hits on {lintr} itself.

@AshesITR
Copy link
Collaborator

LGTM. What do you think about introducing a regex_linter() that can support multiple lints, e.g. from fixed_regex_linter()?
It would allow future regex related lints to be automatically added to projects that opt into regex_linter(), reducing maintenance burden on .lintr.

@MichaelChirico
Copy link
Collaborator Author

There's two avenues we've used for this, one is to have a single linter that handles a lot of things (paste_linter() is quintessential here), the other is using tags. I prefer the latter in most cases -- adding lots of parameters for turning certain lints on/off within a linter is somewhat painful for maintenance.

@AshesITR
Copy link
Collaborator

SGTM. Should we add a tag "regex", then?

@codecov-commenter
Copy link

codecov-commenter commented Nov 16, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1d84cae) 99.45% compared to head (3070d9f) 99.45%.

❗ Current head 3070d9f differs from pull request most recent head 07a7406. Consider uploading reports for the commit 07a7406 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2281   +/-   ##
=======================================
  Coverage   99.45%   99.45%           
=======================================
  Files         117      117           
  Lines        5341     5341           
=======================================
  Hits         5312     5312           
  Misses         29       29           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

R/linter_tag_docs.R Outdated Show resolved Hide resolved
AshesITR
AshesITR previously approved these changes Nov 17, 2023
AshesITR
AshesITR previously approved these changes Nov 18, 2023
@IndrajeetPatil
Copy link
Collaborator

@MichaelChirico This is also ready for a merge after the merge conflict is resolved.

@MichaelChirico MichaelChirico merged commit 6634664 into main Nov 18, 2023
20 checks passed
@MichaelChirico MichaelChirico deleted the which_grepl branch November 18, 2023 21:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants