Change CLI file parsing errors to use an error code#4502
Conversation
boneskull
left a comment
There was a problem hiding this comment.
thanks Evaline! I have some comments/suggestions... please let me know what you think.
|
There's a conflict in |
|
in case anyone else was wondering, I think "unparsable" is correct and "unparseable" is not. my spell checker dislikes both 😄 |
|
I'm not really sure if there is/was a temporary problem with the browser tests since I saw at least another PR with the same failure. At the same time, I don't know how to just re-run the test. 🤔 |
|
hmm. I'll look into that error. some saucelabs weirdness.. hope it's just temporary. this seems to have a conflict now--could you please rebase onto |
|
hmm I'm not seeing a conflict currently, after I merged from
|
|
After a long hiatus I have tried to formally rebase - looks like the browser test on forked PRs problem is known and still existing |
|
You can ignore the browser test. We are working on #4553 . |
juergba
left a comment
There was a problem hiding this comment.
@evaline-ju thank you for this PR.
Description of the Change
After the implementations of #3467 and #3666 and #4464, there are a few instances in the code that do not use error codes yet.
Alternate Designs
Open to other names for this error code
Why should this be in core?
Using codes is a best practice like in Node.js, codes help identify errors as mocha errors at a glance
Benefits
Using codes is a best practice like in Node.js, codes help identify errors as mocha errors at a glance
Possible Drawbacks
None I'm aware of
Applicable issues
Contributes to #3656, semver-minor