-
Notifications
You must be signed in to change notification settings - Fork 177
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: use errors.New to replace fmt.Errorf with no parameters #2320
base: main
Are you sure you want to change the base?
Conversation
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, thanks for the contribution!
Thank you for the review! |
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.
Can you please explain why the changes (in your commit message)? Why is it better to use errors.New instead of fmt.Errorf? Specially because I can see that on the files you added it, the fmt
package is still being imported, so not much gain there.
If it's just a linting thing, then please create a linting rule to avoid people using fmt.Errorf in the future when errors.New would be prefered, otherwise one would need to create this PR again to fix the few new instances
errors.New has better performance, although slightly, since it does not involve the overhead of formatting the string. It is also more concise when there are no formatting arguments. |
Additionally, I noticed that in most projects I read, errors without formatting parameters are typically handled using errors.New. |
Signed-off-by: ChengenH <hce19970702@gmail.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2320 +/- ##
==========================================
+ Coverage 74.38% 74.41% +0.02%
==========================================
Files 109 109
Lines 11493 11493
==========================================
+ Hits 8549 8552 +3
+ Misses 2278 2277 -1
+ Partials 666 664 -2 ☔ View full report in Codecov by Sentry. |
No description provided.