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

assume glue is glue::glue #2032

Merged
merged 5 commits into from Aug 2, 2023
Merged

assume glue is glue::glue #2032

merged 5 commits into from Aug 2, 2023

Conversation

MichaelChirico
Copy link
Collaborator

Without this, #2031 throws false positive lints.

I'm not sure why we didn't do this to begin with.

  • If someone's using a function called glue() that doesn't come from {glue}, they should set interpret_glue = FALSE.
  • If they're using two glue() functions, one of which is unrelated to glue::glue()
    • That's weird, why would you do that?
    • Even so, the only (?) implication is that we'll try and interpret the string input, if there is one, which
      • I guess has a small efficiency impact
      • Shouldn't error
      • Could conceivably cause a false negative if that glue() mask also uses {-interpretation-like formatting that happens to look exactly like a glue::glue() interpolation? Which again, why

AshesITR
AshesITR previously approved these changes Aug 2, 2023
@codecov-commenter
Copy link

codecov-commenter commented Aug 2, 2023

Codecov Report

Merging #2032 (6fb9fb9) into main (2b62a3a) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 6fb9fb9 differs from pull request most recent head 105d814. Consider uploading reports for the commit 105d814 to get more accurate results

@@            Coverage Diff             @@
##             main    #2032      +/-   ##
==========================================
- Coverage   98.55%   98.55%   -0.01%     
==========================================
  Files         113      113              
  Lines        4995     4994       -1     
==========================================
- Hits         4923     4922       -1     
  Misses         72       72              
Files Changed Coverage Δ
R/object_usage_linter.R 99.41% <ø> (-0.01%) ⬇️

@MichaelChirico MichaelChirico merged commit 37e9c39 into main Aug 2, 2023
21 checks passed
@MichaelChirico MichaelChirico deleted the assume-glue branch August 2, 2023 20:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants