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

look for is.unsorted() behavior in sort_linter #2076

Merged
merged 5 commits into from
Aug 13, 2023
Merged

look for is.unsorted() behavior in sort_linter #2076

merged 5 commits into from
Aug 13, 2023

Conversation

MichaelChirico
Copy link
Collaborator

Closes #2072, part of #884

@codecov-commenter
Copy link

codecov-commenter commented Aug 10, 2023

Codecov Report

Merging #2076 (26eba6f) into main (3d9e6d7) will increase coverage by 0.00%.
The diff coverage is 100.00%.

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

@@           Coverage Diff           @@
##             main    #2076   +/-   ##
=======================================
  Coverage   99.64%   99.64%           
=======================================
  Files         117      117           
  Lines        5106     5117   +11     
=======================================
+ Hits         5088     5099   +11     
  Misses         18       18           
Files Changed Coverage Δ
R/sort_linter.R 100.00% <100.00%> (ø)

AshesITR
AshesITR previously approved these changes Aug 11, 2023
@MichaelChirico
Copy link
Collaborator Author

Ah that's annoying.

  • Merging in the Web UI doesn't dismiss a review unless there's a conflict
  • Merging locally & pushing does dismiss a review, even if it was a clean merge
  • The Web UI ignores our .gitattributes

So I can cleanly merge a simple NEWS update locally, but not on the web without dismissing the review. Alas.

@@ -23,6 +23,7 @@
+ `yoda_test_linter()`
* `sprintf_linter()` is pipe-aware, so that `x %>% sprintf(fmt = "%s")` no longer lints (#1943, @MichaelChirico).
* `line_length_linter()` helpfully includes the line length in the lint message (#2057, @MichaelChirico).
* `sort_linter()` checks for code like `x == sort(x)` which is better served by using the function `is.unsorted()` (part of #884, @MichaelChirico).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is pretty cool! I had no idea that this function existed 🙈

@AshesITR AshesITR merged commit 3680289 into main Aug 13, 2023
@AshesITR AshesITR deleted the unsorted branch August 13, 2023 07:13
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.

is_unsorted: new linter, or part of sort_linter() ?
4 participants