-
Notifications
You must be signed in to change notification settings - Fork 795
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
chore: Fix lint warnings in core package #2405
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2405 +/- ##
==========================================
- Coverage 92.64% 92.64% -0.01%
==========================================
Files 137 137
Lines 4974 4973 -1
Branches 1047 1047
==========================================
- Hits 4608 4607 -1
Misses 366 366
|
4ba6502
to
8fe9f66
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.
Imho such contributions are invaluable, thanks!
One question though: Any specific reason to replace const
function declarations with function
statements?
No, I just saw using |
@@ -129,6 +130,7 @@ export function hrTimeToTimeStamp(hrTime: api.HrTime): string { | |||
* Convert hrTime to nanoseconds. | |||
* @param hrTime | |||
*/ | |||
// eslint-disable-next-line @typescript-eslint/no-shadow |
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 think the no-shadow
error should be rather fixed then ignored, in most cases this should be an easy fix
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 have answered this in another comment as well. I couldn't find a better name for the parameter and thought the name of the parameter should be meaningful for users who don't use types.
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 be fine with just using time
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.
Fixed.
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.
The parameter name is only ever likely to show up in intellisense and generated docs. In both cases it woudl be alongside its type which is clearly named.
I updated the PR title. Please in the future use conventional commits for titles since they become the commit message when PRs are squashed. |
related to #1093,
Which problem is this PR solving?
core
package.Short description of the changes