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

KUBESAW-250: Updating GolangCiLint to v1.63.1 #1116

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

fbm3307
Copy link
Contributor

@fbm3307 fbm3307 commented Jan 7, 2025

MatousJobanek
MatousJobanek previously approved these changes Jan 8, 2025
Signed-off-by: Feny Mehta <[email protected]>
Copy link
Contributor Author

@fbm3307 fbm3307 left a comment

Choose a reason for hiding this comment

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

The places where i have written // nolint:gosec , it bcoz the linter from 1.60 and above are throwing integer overflow conversion uint->Int and vice versa error , and the fix they have provided seems not working for us, probably, the long term solutions would be to fix this overflow error by not having to convert and only have one ttype of var

pkg/capacity/manager.go Show resolved Hide resolved
Signed-off-by: Feny Mehta <[email protected]>
Copy link
Contributor

@MatousJobanek MatousJobanek left a comment

Choose a reason for hiding this comment

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

please fix the comments

pkg/capacity/manager.go Outdated Show resolved Hide resolved
pkg/capacity/manager.go Outdated Show resolved Hide resolved
@MatousJobanek MatousJobanek self-requested a review January 9, 2025 13:34
@MatousJobanek MatousJobanek dismissed their stale review January 9, 2025 13:35

dismissing the approval as there are still things to address

Signed-off-by: Feny Mehta <[email protected]>
pkg/capacity/manager.go Outdated Show resolved Hide resolved
pkg/capacity/manager.go Outdated Show resolved Hide resolved
@fbm3307 fbm3307 requested a review from MatousJobanek January 10, 2025 13:25
Signed-off-by: Feny Mehta <[email protected]>
@@ -46,7 +46,8 @@ func NewSpaceWithFeatureToggles(userSignup *toolchainv1alpha1.UserSignup, target
func addFeatureToggles(space *toolchainv1alpha1.Space, toggles []toolchainconfig.FeatureToggle) {
var winners []string
for _, t := range toggles {
weight := int(t.Weight())
//the value of weight is not expected to go beyond 100 or negative , it won't overflow, hence its okay to ignore the overflow linter error
Copy link
Contributor

Choose a reason for hiding this comment

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

you are converting from uint (which cannot be negative on it's own) to int

Suggested change
//the value of weight is not expected to go beyond 100 or negative , it won't overflow, hence its okay to ignore the overflow linter error
//the value of weight is not expected to go beyond 100, it won't overflow, hence its okay to ignore the overflow linter error

Copy link

openshift-ci bot commented Jan 10, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: fbm3307, MatousJobanek

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [MatousJobanek,fbm3307]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Copy link

codecov bot commented Jan 10, 2025

Codecov Report

Attention: Patch coverage is 72.22222% with 5 lines in your changes missing coverage. Please review.

Project coverage is 79.15%. Comparing base (69a195d) to head (a3cb3c2).

Files with missing lines Patch % Lines
main.go 0.00% 3 Missing ⚠️
controllers/space/space_controller.go 0.00% 1 Missing ⚠️
controllers/usersignup/usersignup_controller.go 50.00% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1116   +/-   ##
=======================================
  Coverage   79.15%   79.15%           
=======================================
  Files          78       78           
  Lines        7809     7814    +5     
=======================================
+ Hits         6181     6185    +4     
- Misses       1449     1450    +1     
  Partials      179      179           
Files with missing lines Coverage Δ
...isionerconfig/spaceprovisionerconfig_controller.go 83.21% <100.00%> (+0.24%) ⬆️
...lers/toolchainconfig/toolchainconfig_controller.go 82.75% <100.00%> (ø)
...lers/toolchainstatus/toolchainstatus_controller.go 88.92% <100.00%> (ø)
pkg/capacity/manager.go 98.50% <100.00%> (+0.02%) ⬆️
pkg/space/space.go 96.20% <100.00%> (+0.04%) ⬆️
controllers/space/space_controller.go 85.53% <0.00%> (ø)
controllers/usersignup/usersignup_controller.go 75.15% <50.00%> (ø)
main.go 0.00% <0.00%> (ø)

... and 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants