Skip to content
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

fix golangci-lint issues #3679

Merged
merged 9 commits into from
Jun 11, 2024
Merged

fix golangci-lint issues #3679

merged 9 commits into from
Jun 11, 2024

Conversation

dmitris
Copy link
Contributor

@dmitris dmitris commented May 10, 2024

PR is the continuation of work on #3525 and a follow-up on #3621. It fixes a number of the issues flagged by golangci-lint run -D errcheck -D unused -D ineffassign --max-same-issues 0 ./... though not yet all of them ("eating the elephant piece by piece" 😄 )

@dmitris dmitris changed the title fix golangci-lint issue fix golangci-lint issues May 10, 2024
hhhjort
hhhjort previously approved these changes May 18, 2024
Copy link
Collaborator

@hhhjort hhhjort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@AlexBVolcy AlexBVolcy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left some comments. Could you resolve the TODOs as well please?

adapters/consumable/consumable.go Outdated Show resolved Hide resolved
adapters/grid/grid.go Outdated Show resolved Hide resolved
@dmitris
Copy link
Contributor Author

dmitris commented May 23, 2024

Left some comments. Could you resolve the TODOs as well please?

AlexBVolcy the remaining TODOs removed in 4036d50.

adapters/mobfoxpb/mobfoxpb.go Outdated Show resolved Hide resolved
adapters/grid/grid.go Outdated Show resolved Hide resolved
@dmitris
Copy link
Contributor Author

dmitris commented Jun 3, 2024

The output of the command golangci-lint run -D errcheck -D unused -D ineffassign --max-same-issues 0 ./... on this branch is below (copy in gist). I would suggest to merge this PR before making other changes.

adapters/rubicon/rubicon.go:393:4: SA4006: this value of `err` is never used (staticcheck)
			videoCopy.Ext, err = json.Marshal(&videoExt)
			^
adapters/rubicon/rubicon.go:456:4: SA4006: this value of `err` is never used (staticcheck)
			siteCopy.Publisher.Ext, err = json.Marshal(&pubExt)
			^
analytics/agma/sender_test.go:125:2: SA4006: this value of `err` is never used (staticcheck)
	sender, err := createHttpSender(client, config.AgmaAnalyticsHttpEndpoint{
	^
prebid_cache_client/client.go:121:2: SA4006: this value of `err` is never used (staticcheck)
	responseBody, err := io.ReadAll(anResp.Body)
	^
adapters/adgeneration/adgeneration_test.go:248:5: SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
	if bidderResponse == nil {
	   ^
adapters/adgeneration/adgeneration_test.go:264:40: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, 1, len(bidderResponse.Bids))
	                                      ^
adapters/adgeneration/adgeneration_test.go:265:45: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedID, bidderResponse.Bids[0].Bid.ID)
	                                           ^
adapters/adgeneration/adgeneration_test.go:266:48: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedImpID, bidderResponse.Bids[0].Bid.ImpID)
	                                              ^
adapters/adgeneration/adgeneration_test.go:267:46: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedAdM, bidderResponse.Bids[0].Bid.AdM)
	                                            ^
adapters/adgeneration/adgeneration_test.go:268:48: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedPrice, bidderResponse.Bids[0].Bid.Price)
	                                              ^
adapters/adgeneration/adgeneration_test.go:269:44: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedW, bidderResponse.Bids[0].Bid.W)
	                                          ^
adapters/adgeneration/adgeneration_test.go:270:44: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedH, bidderResponse.Bids[0].Bid.H)
	                                          ^
adapters/adgeneration/adgeneration_test.go:271:47: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, expectedCrID, bidderResponse.Bids[0].Bid.CrID)
	                                             ^
adapters/adgeneration/adgeneration_test.go:272:49: SA5011: possible nil pointer dereference (staticcheck)
	assert.Equal(t, extectedDealID, bidderResponse.Bids[0].Bid.DealID)
	                                               ^
exchange/bidder_test.go:1069:58: SA5011: possible nil pointer dereference (staticcheck)
		assert.Equal(t, tc.expectedBidsCount, uint(len(seatBid.Bids)), tc.description)
		                                                       ^
exchange/bidder_test.go:1068:20: SA5011(related information): this check suggests that the pointer can be nil (staticcheck)
		assert.Falsef(t, seatBid == nil && tc.expectedBidsCount != 0, tc.description)
		                 ^
stored_requests/events/http/http.go:83:15: SA1015: using time.Tick leaks the underlying ticker, consider using it only in endless functions, tests and the main package, and use time.NewTicker here (staticcheck)
	go e.refresh(time.Tick(refreshRate))
	             ^
adapters/yandex/yandex.go:159:40: SA1019: yandexExt.ImpID is deprecated: in favor of `PlacementID` (staticcheck)
		placementID.ImpID = strconv.Itoa(int(yandexExt.ImpID))
		                                     ^
adapters/yandex/yandex.go:160:41: SA1019: yandexExt.PageID is deprecated: in favor of `PlacementID` (staticcheck)
		placementID.PageID = strconv.Itoa(int(yandexExt.PageID))
		                                      ^
server/ssl/ssl_test.go:15:14: SA1019: certPool.Subjects has been deprecated since Go 1.18: if s was returned by [SystemCertPool], Subjects will not include the system roots. (staticcheck)
	subjects := certPool.Subjects()
	            ^
server/ssl/ssl_test.go:25:13: SA1019: certPool.Subjects has been deprecated since Go 1.18: if s was returned by [SystemCertPool], Subjects will not include the system roots. (staticcheck)
	subjects = certPool.Subjects()
	           ^
server/ssl/ssl_test.go:40:14: SA1019: certPool.Subjects has been deprecated since Go 1.18: if s was returned by [SystemCertPool], Subjects will not include the system roots. (staticcheck)
	subjects := certPool.Subjects()
	            ^

Copy link
Collaborator

@hhhjort hhhjort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@bsardo bsardo merged commit 1979036 into prebid:master Jun 11, 2024
5 checks passed
@dmitris dmitris deleted the golangci-lint02 branch June 11, 2024 19:35
mefjush pushed a commit to adhese/prebid-server that referenced this pull request Jul 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants