-
Notifications
You must be signed in to change notification settings - Fork 2.8k
chore: enable several rules from go-critic #7430
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: enable several rules from go-critic #7430
Conversation
8a1aede to
89ff3a1
Compare
| - pkg: github.com/jaegertracing/jaeger/internal/proto-gen/api_v2$ | ||
| desc: Use github.com/jaegertracing/jaeger-idl/proto-gen/api_v2 | ||
| gocritic: | ||
| disabled-checks: |
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 is great way to see which linters we are not running. Some of them look useful to enable.
| var queryResponse response | ||
| if err = json.Unmarshal(body, &queryResponse); err != nil { | ||
| err = json.Unmarshal(body, &queryResponse) | ||
| if err != nil { |
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.
Keeping the original but making err scoped locally with err := would be a better 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.
Yes but then you have shadow error from govet
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 rename
if err1 := json.Unmarshal(body, &queryResponse); err1 != nil {
return nil, err1
}
|
Unit tests failed |
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
Signed-off-by: Matthieu MOREL <matthieu.morel35@gmail.com>
89ff3a1 to
a257880
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7430 +/- ##
=======================================
Coverage 96.46% 96.46%
=======================================
Files 377 377
Lines 23071 23084 +13
=======================================
+ Hits 22255 22268 +13
Misses 617 617
Partials 199 199
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:npm run lintandnpm run test