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

Add test for native pipe in commented_code_linter() #1799

Merged
merged 4 commits into from
Dec 5, 2022

Conversation

IndrajeetPatil
Copy link
Collaborator

No description provided.

@AshesITR
Copy link
Collaborator

AshesITR commented Dec 5, 2022

Why would commented_code_linter() need more tests regarding the pipe?

@codecov-commenter
Copy link

codecov-commenter commented Dec 5, 2022

Codecov Report

Merging #1799 (ea9c1f0) into main (c833f8a) will not change coverage.
The diff coverage is n/a.

❗ Current head ea9c1f0 differs from pull request most recent head ff63b2e. Consider uploading reports for the commit ff63b2e to get more accurate results

@@           Coverage Diff           @@
##             main    #1799   +/-   ##
=======================================
  Coverage   98.89%   98.89%           
=======================================
  Files         110      110           
  Lines        4692     4692           
=======================================
  Hits         4640     4640           
  Misses         52       52           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@IndrajeetPatil
Copy link
Collaborator Author

Why would commented_code_linter() need more tests regarding the pipe?

Because, currently, we are only testing the magrittr pipe, and not the native pipe.

@AshesITR
Copy link
Collaborator

AshesITR commented Dec 5, 2022

But from a code analysis point-of-view, all the SPECIALs are equivalent, i.e. %anything% and %>% are essentially the same thing for the parser.
I'm fine with adding a |> test just-in-case, but the others seem excessive.

@IndrajeetPatil
Copy link
Collaborator Author

@AshesITR Fair enough! I've reverted changes to the ops vector that included additional infix ops.

Copy link
Collaborator

@AshesITR AshesITR left a comment

Choose a reason for hiding this comment

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

Thanks 😊

@AshesITR AshesITR merged commit 5877d70 into main Dec 5, 2022
@AshesITR AshesITR deleted the commented_code_linter_pipe branch December 5, 2022 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants