-
Notifications
You must be signed in to change notification settings - Fork 637
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
refactor(archive,cache,datetime,fmt,front-matter): align error messages to the style guide #5706
Conversation
Aligns the error messages in the `front_matter` folder to match the style guide. denoland#5574
Aligns the error messages in the `cache` folder to match the style guide. denoland#5574
Aligns the error messages in the `archive` folder to match the style guide. denoland#5574
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5706 +/- ##
==========================================
- Coverage 96.28% 96.25% -0.04%
==========================================
Files 474 474
Lines 38383 38403 +20
Branches 5578 5586 +8
==========================================
+ Hits 36957 36964 +7
- Misses 1384 1397 +13
Partials 42 42 ☔ View full report in Codecov by Sentry. |
Aligns the error messages in the `datetime` folder to match the style guide. denoland#5574
Aligns the error messages in the `fmt` folder to match the style guide. denoland#5574
fmt/printf.ts
Outdated
@@ -611,7 +621,10 @@ class Printf { | |||
prefix += "0x"; | |||
break; | |||
default: | |||
throw new Error("cannot handle base: " + radix); | |||
throw new Error( | |||
"Format number only supports radix 2, 8 and 16: cannot handle radix " + |
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'd suggest the below
"Format number only supports radix 2, 8 and 16: cannot handle radix " + | |
`Cannot handle the radix ${radix}. Only 2, 8, 16 are supported` |
because of the rule Message should state the action that lead to the error
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've merged a slightly different version of this that reads: Cannot handle the radix ${radix}: only 2, 8, 16 are supported
fmt/printf.ts
Outdated
@@ -881,7 +898,7 @@ class Printf { | |||
} | |||
default: | |||
throw new Error( | |||
"currently only number and string are implemented for hex", | |||
`Only "number" and "string" are implemented for hex formatting: ${typeof val} is not currently supported`, |
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.
`Only "number" and "string" are implemented for hex formatting: ${typeof val} is not currently supported`, | |
`Cannot format hex. Only number and string are supported for hex formatting: ${typeof val} is given, |
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've committed a slightly different version of this that reads: Cannot format hex, only number and string are supported for hex formatting: ${typeof val} is given
Commit suggestions by @kt3k Co-authored-by: Yoshiya Hinosawa <[email protected]>
Fixed a syntax error
Update test cases to match the new format
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.
LGTM
Aligns error messages in
archive
,cache
,front_matter
,date_time
andfmt
folders.