-
-
Notifications
You must be signed in to change notification settings - Fork 919
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
infra: improve error messages for parameter properties #3082
Conversation
✅ Deploy Preview for fakerjs ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## next #3082 +/- ##
========================================
Coverage 99.95% 99.96%
========================================
Files 2776 2776
Lines 226260 226260
Branches 587 939 +352
========================================
+ Hits 226164 226185 +21
+ Misses 96 75 -21 |
331c196
to
1e6b792
Compare
This reverts commit c06519f.
Any wishes regarding the two error message parts (mainText + causeText)? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would potentially still be a bit confused when I would encounter such a causeText
, but I see that you just pass forward the error.message
. So for now this is fine for me.
The causeText is just defined in a different location.
Would the following be easier to understand? `Expected exactly one ${property} element, got ${input.length}`
`Expected exactly one jsdoc element, got 0` (I have to check the other usages still) |
I think the real confusion is that it is not intuitive that you have to write the JSDocs for the option parameters twice, once at the top of the method and once directly above each option. So this is an improvement, but still not sure that a new contributor confronted with this message would know how to fix it. |
How about an error like this:
|
This is BY FAR EXORBITANT much better 😍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work @ST-DDT 👏
Partially fixes #3079
Adds distinct error references for option parameter's properties and outputs them to the github action errors.