Skip to content

Conversation

@mmorel-35
Copy link
Contributor

Which problem is this PR solving?

Description of the changes

  • Enable all rules by default and disable the rules not yet applied

How was this change tested?

Checklist

@mmorel-35 mmorel-35 force-pushed the gocritic/enable-all branch 5 times, most recently from 8a1aede to 89ff3a1 Compare August 11, 2025 06:42
@mmorel-35 mmorel-35 marked this pull request as ready for review August 11, 2025 06:48
@mmorel-35 mmorel-35 requested a review from a team as a code owner August 11, 2025 06:48
@mmorel-35 mmorel-35 requested a review from joe-elliott August 11, 2025 06:48
@dosubot dosubot bot added the go label Aug 11, 2025
- pkg: github.com/jaegertracing/jaeger/internal/proto-gen/api_v2$
desc: Use github.com/jaegertracing/jaeger-idl/proto-gen/api_v2
gocritic:
disabled-checks:
Copy link
Member

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 {
Copy link
Member

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

Copy link
Contributor Author

@mmorel-35 mmorel-35 Aug 11, 2025

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

Copy link
Member

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
	}

@yurishkuro yurishkuro added the changelog:refactoring Internal code refactoring without functional changes label Aug 11, 2025
@yurishkuro
Copy link
Member

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>
@mmorel-35 mmorel-35 force-pushed the gocritic/enable-all branch from 89ff3a1 to a257880 Compare August 11, 2025 12:42
@codecov
Copy link

codecov bot commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 95.08197% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.46%. Comparing base (a7e59dd) to head (a257880).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
internal/storage/kafka/consumer/config.go 0.00% 0 Missing and 1 partial ⚠️
internal/storage/kafka/producer/config.go 0.00% 0 Missing and 1 partial ⚠️
internal/storage/v1/elasticsearch/factory.go 50.00% 0 Missing and 1 partial ⚠️
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           
Flag Coverage Δ
badger_v1 9.03% <31.25%> (+0.02%) ⬆️
badger_v2 1.70% <0.00%> (-0.01%) ⬇️
cassandra-4.x-v1-manual 11.69% <0.00%> (-0.01%) ⬇️
cassandra-4.x-v2-auto 1.69% <0.00%> (-0.01%) ⬇️
cassandra-4.x-v2-manual 1.69% <0.00%> (-0.01%) ⬇️
cassandra-5.x-v1-manual 11.69% <0.00%> (-0.01%) ⬇️
cassandra-5.x-v2-auto 1.69% <0.00%> (-0.01%) ⬇️
cassandra-5.x-v2-manual 1.69% <0.00%> (-0.01%) ⬇️
elasticsearch-6.x-v1 16.60% <3.12%> (-0.01%) ⬇️
elasticsearch-7.x-v1 16.64% <3.12%> (-0.01%) ⬇️
elasticsearch-8.x-v1 16.78% <3.12%> (-0.01%) ⬇️
elasticsearch-8.x-v2 1.70% <0.00%> (-0.01%) ⬇️
elasticsearch-9.x-v2 1.70% <0.00%> (-0.01%) ⬇️
grpc_v1 10.22% <3.12%> (-0.01%) ⬇️
grpc_v2 1.70% <0.00%> (-0.01%) ⬇️
kafka-3.x-v1 9.68% <9.37%> (-0.01%) ⬇️
kafka-3.x-v2 1.70% <0.00%> (-0.01%) ⬇️
memory_v2 1.70% <0.00%> (-0.01%) ⬇️
opensearch-1.x-v1 16.69% <3.12%> (-0.01%) ⬇️
opensearch-2.x-v1 16.69% <3.12%> (-0.01%) ⬇️
opensearch-2.x-v2 1.70% <0.00%> (-0.01%) ⬇️
opensearch-3.x-v2 1.70% <0.00%> (-0.01%) ⬇️
query 1.70% <0.00%> (-0.01%) ⬇️
tailsampling-processor 0.47% <0.00%> (-0.01%) ⬇️
unittests 95.45% <95.08%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@yurishkuro yurishkuro merged commit a5de984 into jaegertracing:main Aug 11, 2025
63 checks passed
@mmorel-35 mmorel-35 deleted the gocritic/enable-all branch August 11, 2025 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog:refactoring Internal code refactoring without functional changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants