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

test: Enable a few good typescript-eslint rules #969

Merged
merged 5 commits into from May 13, 2022

Conversation

samestep
Copy link
Collaborator

@samestep samestep commented May 13, 2022

Description

This PR enables a few good typescript-eslint rules:

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new ESLint warnings
  • I have reviewed any generated changes to the diagrams/ folder

@samestep samestep requested a review from wodeni May 13, 2022 04:02
@cloudflare-pages
Copy link

cloudflare-pages bot commented May 13, 2022

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: d3e2ead
Status:⚡️  Build in progress...

View logs

@codecov
Copy link

codecov bot commented May 13, 2022

Codecov Report

Merging #969 (d3e2ead) into main (ea8d9f4) will increase coverage by 0.07%.
The diff coverage is 58.06%.

@@            Coverage Diff             @@
##             main     #969      +/-   ##
==========================================
+ Coverage   63.23%   63.30%   +0.07%     
==========================================
  Files          62       62              
  Lines        7918     7906      -12     
  Branches     1837     1829       -8     
==========================================
- Hits         5007     5005       -2     
+ Misses       2790     2781       -9     
+ Partials      121      120       -1     
Impacted Files Coverage Δ
packages/core/src/contrib/Utils.ts 31.03% <0.00%> (+1.52%) ⬆️
packages/core/src/renderer/Path.ts 10.12% <0.00%> (+0.12%) ⬆️
packages/core/src/synthesis/Synthesizer.ts 28.19% <0.00%> (+0.06%) ⬆️
packages/core/src/utils/Util.ts 54.78% <0.00%> (+0.47%) ⬆️
packages/core/src/utils/CollectLabels.ts 66.31% <60.00%> (+0.33%) ⬆️
packages/core/src/compiler/Style.ts 78.88% <100.00%> (-0.05%) ⬇️
packages/core/src/engine/Autodiff.ts 81.85% <100.00%> (+0.03%) ⬆️
packages/core/src/engine/EngineUtils.ts 64.96% <100.00%> (ø)
packages/core/src/engine/Evaluator.ts 33.33% <100.00%> (+0.35%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ea8d9f4...d3e2ead. Read the comment docs.

@samestep samestep requested a review from cmumatt May 13, 2022 15:09
Copy link
Contributor

@cmumatt cmumatt left a comment

Choose a reason for hiding this comment

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

Looks amazing -- I love these kinds of changes! Thanks for doing this hard work to improve the codebase now and in the future. Do we need to log an issue so we go back and clean up any remaining warnings and change the mode from warning to error?

@samestep
Copy link
Collaborator Author

samestep commented May 13, 2022

Do we need to log an issue so we go back and clean up any remaining warnings and change the mode from warning to error?

Hmm we can; the thing is, a lot of the things we have set to "warning" would take a significant amount of refactoring effort to change, because they result from systematic typing deficiencies in our architecture. If we do end up resolving them to the degree that we can actually change the mode to "error", that would just be a side effect of a much larger effort.

@samestep samestep merged commit ae6ace3 into main May 13, 2022
@samestep samestep deleted the enable-typescript-eslint-rules branch May 13, 2022 16:56
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

2 participants