-
Notifications
You must be signed in to change notification settings - Fork 336
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
Add common CLI configuration error categories #2130
Conversation
Suggested in github#2123 but was automerged before I could apply the suggestion
This clashes with the logic for the `NoJavaScriptTypeScriptCodeFound` case. Once that situation is deprecated, we can add the exit code back to `DetectedCodeButCouldNotProcess` configuration.
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.
Nice! A couple of suggestions:
be36887
to
8361861
Compare
Co-authored-by: Henry Mercer <[email protected]>
These tests will also result in the "No code found" error, but the functionality they test is unrelated, so we simply match against the bits we care about.
Except in the special case where we prepend the docs link, which is handled separately
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.
Looks good, just a couple of final comments.
fc1bd33
to
0bec49a
Compare
Co-authored-by: Henry Mercer <[email protected]>
0bec49a
to
3dc47a2
Compare
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.
We looked through recent telemetry errors and these were some of the most common failures we attribute to configuration errors. I've added them now that we have a nice place to do so!
Also, a couple of tiny drive-by improvements that I didn't think merited their own PRs.
Merge / deployment checklist