-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
mention that a comma might be expected #13650
base: main
Are you sure you want to change the base?
Conversation
⏱️ 1h 2m total CI duration on this PR
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #13650 +/- ##
=======================================
Coverage 58.9% 59.0%
=======================================
Files 824 824
Lines 198320 198327 +7
=======================================
+ Hits 117008 117015 +7
Misses 81312 81312 ☔ View full report in Codecov by Sentry. |
│ - ^ Expected '>' | ||
│ - ^ Expected either ',' or '>' |
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.
This one is a bit weird, because it's just that phatom is not a keyword
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.
Error messages are hard, as you see.
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.
But it's not really terrible, and this is a small improvement in almost all cases, so maybe just approve?
376b88c
to
c64f6f4
Compare
c64f6f4
to
5233817
Compare
This issue is stale because it has been open 45 days with no activity. Remove the |
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.
Just leaving a test top level comment.
This issue is stale because it has been open 45 days with no activity. Remove the |
This issue is stale because it has been open 45 days with no activity. Remove the |
Description
In case of a missing comma, mention that as part of an error message.
Fixes #13632.
How Has This Been Tested?
Added a test case based on the issue. See an existing test case also is updated.
Key Areas to Review
Is it really clearer?
Type of Change
Which Components or Systems Does This Change Impact?
Checklist