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: do not have special handling for top level errors #74

Merged
merged 2 commits into from
Nov 4, 2024

Conversation

sandhya-spend
Copy link
Contributor

There were two problems with earlier implementation

  • errors[''] does not have any special meaning, so should not be handled separately
  • Also, errors should be of the form Record<string, string[]> but was implemented as errors[''] = <string>

@decrapifier
Copy link
Contributor

Fails
🚫 Please assign someone to merge this PR, and optionally include people who should review.
Warnings
⚠️ You should add at least 1 more reviewer to the PR
Messages
📖 We were able to automatically fix some formatting issues in this PR for you!

Coverage Report

👍 Test coverage is looking good.

Files % Stmts % Branch % Funcs % Lines Uncovered Lines
src/futil.js 100 100 100 100
src/index.js 100 96.61 96.67 100
src/mobx.js 94.87 88.24 100 94.87 10, 11
and 2 more...
Files % Stmts % Branch % Funcs % Lines Uncovered Lines
src/util.js 100 100 100 100
src/validators.js 52.38 100 50 52.38 6, 7, 8, 9, 10, 11, 12, 13, 14, 15

Some things that were possibly fixed:

  • Code that could be fixed via the --fix flag
  • Formatting that could be fixed by prettier

Take a look at this commit to see what happened in detail: 0838629

And look at this wiki page to see the reasoning behind the ESLint rules: https://github.com/smartprocure/eslint-config-smartprocure/wiki/Rules-and-Why-We-Chose-Them

Generated by 🚫 dangerJS against 3d3a463

@decrapifier
Copy link
Contributor

Fails
🚫 Please assign someone to merge this PR, and optionally include people who should review.
Warnings
⚠️ You should add at least 1 more reviewer to the PR

Coverage Report

👍 Test coverage is looking good.

Files % Stmts % Branch % Funcs % Lines Uncovered Lines
src/futil.js 100 100 100 100
src/index.js 100 96.61 96.67 100
src/mobx.js 94.87 88.24 100 94.87 10, 11
and 2 more...
Files % Stmts % Branch % Funcs % Lines Uncovered Lines
src/util.js 100 100 100 100
src/validators.js 52.38 100 50 52.38 6, 7, 8, 9, 10, 11, 12, 13, 14, 15

Generated by 🚫 dangerJS against 0838629

@sandhya-spend sandhya-spend self-assigned this Nov 4, 2024
@sandhya-spend sandhya-spend merged commit 2690c33 into master Nov 4, 2024
1 of 2 checks passed
@stellarhoof stellarhoof deleted the fix/generic_error_handling branch November 4, 2024 22:18
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.

3 participants