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

refactor: resolve eslint warnings (complexity) - src/lib/filter/filterNoMatchReason.ts #1585

Merged
merged 6 commits into from
Dec 17, 2024

Conversation

btlghrants
Copy link
Collaborator

Describe what should be investigated or refactored

Refactor of src/lib/filter/filterNoMatchReason.ts to reduce complexity warnings:

/pepr/src/lib/filter/filterNoMatchReason.ts
  32:8  warning  Function 'filterNoMatchReason' has a complexity of 13. Maximum allowed is 10  complexity

Additional context

Fixes #1584
In support of #1248

@btlghrants btlghrants self-assigned this Dec 16, 2024
@btlghrants btlghrants added this to the v0.42.2 milestone Dec 16, 2024
Copy link

codecov bot commented Dec 16, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.13%. Comparing base (cd255d5) to head (95bdf8e).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1585      +/-   ##
==========================================
- Coverage   81.30%   81.13%   -0.18%     
==========================================
  Files          42       41       -1     
  Lines        1952     1945       -7     
  Branches      451      429      -22     
==========================================
- Hits         1587     1578       -9     
+ Misses        363      339      -24     
- Partials        2       28      +26     
Files with missing lines Coverage Δ
src/lib/filter/filter.ts 100.00% <100.00%> (ø)
src/lib/processors/watch-processor.ts 78.64% <100.00%> (ø)

... and 7 files with indirect coverage changes

@btlghrants btlghrants marked this pull request as ready for review December 16, 2024 23:00
@btlghrants btlghrants requested a review from a team as a code owner December 16, 2024 23:00
@btlghrants btlghrants force-pushed the 1584_complex_filternomatchreason branch from e684065 to 546d09d Compare December 17, 2024 12:41
cmwylie19
cmwylie19 previously approved these changes Dec 17, 2024
Copy link
Collaborator

@cmwylie19 cmwylie19 left a comment

Choose a reason for hiding this comment

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

The logic is sound but if we merge in #1364 we are going to be missing many return types:

/pepr/src/lib/filter/filter.ts
   67:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   68:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   69:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   70:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   71:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   72:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   73:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   74:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   75:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   76:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   77:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   78:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   79:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   80:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   81:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   82:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  113:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  114:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  115:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  116:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  117:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  118:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  119:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  120:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  121:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  122:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  123:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  124:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type

@btlghrants
Copy link
Collaborator Author

btlghrants commented Dec 17, 2024

The logic is sound but if we merge in #1364 we are going to be missing many return types:

/pepr/src/lib/filter/filter.ts
   67:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   68:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   69:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   70:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   71:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   72:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   73:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   74:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   75:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   76:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   77:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   78:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   79:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   80:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   81:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
   82:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  113:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  114:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  115:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  116:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  117:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  118:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  119:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  120:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  121:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  122:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  123:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type
  124:8  warning  Missing return type on function  @typescript-eslint/explicit-function-return-type

I think... I do not value the explicit-function-return-type rule and I would love to turn it off: it's a hammer that requires return types even when type inference & local type defs will more-the-suffice (...without making us c&p return type class names that are 75% the length of the tiny, inline funcs we're trying to declare, ugh).

image

That said... I totally forgot that we planned to turn this rule on, so until we can talk about turning it off for-realz, lemme just go add the types back in. Blerg.

Copy link
Collaborator

@cmwylie19 cmwylie19 left a comment

Choose a reason for hiding this comment

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

very nice!

@cmwylie19 cmwylie19 added this pull request to the merge queue Dec 17, 2024
Merged via the queue into main with commit 9aa2930 Dec 17, 2024
48 checks passed
@cmwylie19 cmwylie19 deleted the 1584_complex_filternomatchreason branch December 17, 2024 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

refactor: src/lib/filter/filterNoMatchReason.ts
2 participants