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

fix: eslint max depth warnings #1255

Merged
merged 16 commits into from
Oct 31, 2024

Conversation

tamirazrab
Copy link
Contributor

Description

Fixes eslint max-depth warnings in different files.

Left out helper.ts purposely not sure how to remove nesting there.

Related Issue

Fixes #1249

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Other (security config, docs update, etc)

Checklist before merging

@tamirazrab tamirazrab requested a review from a team as a code owner October 10, 2024 05:00
@cmwylie19
Copy link
Collaborator

We need to hold off on this PR until our release next week. Thank you for your contribution!

Copy link

codecov bot commented Oct 15, 2024

Codecov Report

Attention: Patch coverage is 31.70732% with 28 lines in your changes missing coverage. Please review.

Project coverage is 79.85%. Comparing base (0971bd3) to head (e5bce29).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/lib/watch-processor.ts 35.00% 13 Missing ⚠️
src/lib/mutate-processor.ts 0.00% 12 Missing ⚠️
src/lib/validate-processor.ts 0.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1255      +/-   ##
==========================================
+ Coverage   79.69%   79.85%   +0.16%     
==========================================
  Files          40       40              
  Lines        1945     1941       -4     
  Branches      384      409      +25     
==========================================
  Hits         1550     1550              
+ Misses        393      389       -4     
  Partials        2        2              
Files with missing lines Coverage Δ
src/lib/helpers.ts 97.17% <100.00%> (-0.02%) ⬇️
src/lib/validate-processor.ts 13.51% <0.00%> (+0.35%) ⬆️
src/lib/mutate-processor.ts 9.58% <0.00%> (+0.49%) ⬆️
src/lib/watch-processor.ts 78.64% <35.00%> (-0.57%) ⬇️

src/cli/init/index.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@samayer12 samayer12 left a comment

Choose a reason for hiding this comment

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

Linter warnings are resolved (aside from helper.ts as noted), thank you! Left a couple comments for questions that the maintainers can consider prior to merging.

@cmwylie19
Copy link
Collaborator

can you please run npm run format:fix to try and fix the formatting issues? @tamirazrab

@tamirazrab
Copy link
Contributor Author

can you please run npm run format:fix to try and fix the formatting issues? @tamirazrab

Yes I did, sorry for delay, most warning aren't related to max-depth other than pepr/src/lib/helpers.ts which I'm not really sure how to proceed refactoring it.

@samayer12
Copy link
Collaborator

@tamirazrab, thanks for doing the bulk of the work resolving the max-depth warnings! I'm gonna knock out the last few things and update the branch for us since we've had several changes over the past few weeks.

@samayer12
Copy link
Collaborator

There are broader issues with test-coverage in the refactored files that we already know about, accepting the small overall decrease to gain linter resolutions.

@samayer12 samayer12 merged commit 7453d6a into defenseunicorns:main Oct 31, 2024
40 of 43 checks passed
samayer12 added a commit that referenced this pull request Oct 31, 2024
cmwylie19 pushed a commit that referenced this pull request Oct 31, 2024
btlghrants added a commit that referenced this pull request Nov 4, 2024
This reverts commit 9dbf9ab.

## Description

#1249, but with attention paid to passing E2E tests that were missed in
#1255. Two instances of `max-depth` violations remain in the codebase
(`validate-processor.ts` & `mutate-processor.ts`), but that's a lot
better than where things were.

End to End Test:  
(See [Pepr Excellent
Examples](https://github.com/defenseunicorns/pepr-excellent-examples))

## Related Issue

Relates to #1249 

## Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [x] Other (security config, docs update, etc)

## Checklist before merging
- [x] Unit,
[Journey](https://github.com/defenseunicorns/pepr/tree/main/journey),
[E2E Tests](https://github.com/defenseunicorns/pepr-excellent-examples),
[docs](https://github.com/defenseunicorns/pepr/tree/main/docs),
[adr](https://github.com/defenseunicorns/pepr/tree/main/adr) added or
updated as needed
- [x] [Contributor Guide
Steps](https://docs.pepr.dev/main/contribute/#submitting-a-pull-request)
followed

---------

Co-authored-by: Barrett <81570928+btlghrants@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

Resolve eslint max-depth warnings
3 participants