-
Notifications
You must be signed in to change notification settings - Fork 317
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: upgrade golangci version and lint fixes #3443
Conversation
@@ -12,11 +12,10 @@ Ping returns health check for pg database | |||
func (jd *HandleT) Ping() error { | |||
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) | |||
defer cancel() | |||
rows, err := jd.dbHandle.QueryContext(ctx, `SELECT 'Rudder DB Health Check'::text as message`) | |||
_, err := jd.dbHandle.ExecContext(ctx, `SELECT 'Rudder DB Health Check'::text as message`) |
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.
No need to call QueryContext
if we are not interested in result
@@ -273,7 +273,7 @@ func (t *Stats) GetRouterPickupJobs(destType string, noOfWorkers int, routerTime | |||
break | |||
} | |||
|
|||
pickUpCount := 0 | |||
var pickUpCount int |
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.
linter was complaining: variable defined but got overwritten before used.
@@ -905,6 +905,8 @@ func (d *Deltalake) partitionQuery(ctx context.Context, tableName string) (strin | |||
} | |||
defer func() { _ = rows.Close() }() | |||
|
|||
_ = rows.Err() // ignore 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.
We need to read Columns, but not the rows, no need to check for errors.
I prefer using _ = rows.Err()
, instead of //nolint:rowserrcheck
because it works across all linters.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #3443 +/- ##
==========================================
- Coverage 68.58% 68.51% -0.07%
==========================================
Files 330 330
Lines 52820 52905 +85
==========================================
+ Hits 36224 36248 +24
- Misses 14267 14308 +41
- Partials 2329 2349 +20
☔ View full report in Codecov by Sentry. |
I believe we introduced a couple of panics while handling the errors for the warehouse module. But I think we can take it as a separate PR to handle it. |
My strategy was to mimic the way errors were handled in the function. Otherways the scope of PR could have been much bigger. |
Description
I have upgraded golangci to
version: v1.52
, I have only specified up to minor versions so that patch upgrades will happen automatically.With the upgrade, the following linters were reenabled again (disabled before due to generics)
Linting fixes
rowserrcheck: After
rows.Next()
iteration we need to check forrows.Err()
.wastedassign: Assign and value and not use it. In most cases
x := 0
->var x int
Notion Ticket
https://www.notion.so/rudderstacks/Upgrade-golangci-version-lint-fixes-7abe589fec944d16b16ca378363ac8d1?pvs=4
Security