Skip to content

build: address all lint issues by v2 #20804

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 11 commits into from
May 24, 2025
Merged

build: address all lint issues by v2 #20804

merged 11 commits into from
May 24, 2025
+546 −517

Conversation

LJTian
Copy link
Contributor

@LJTian LJTian commented May 20, 2025 •
edited
Loading

All fixes have been preliminarily completed. Since this PR modifies too many files, it is recommended to review it through Commit records.

fix #20740

@LJTian
fix QF1011: could omit type *os.File from declaration; it will be inf…
Loading
Loading status checks…
a846e29 
…erred from the right-hand side
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label May 20, 2025
@k8s-ci-robot
Copy link
Contributor

Hi @LJTian. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

@k8s-ci-robot k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 20, 2025
@minikube-bot
Copy link
Collaborator

Can one of the admins verify this patch?

@LJTian
Copy link
Contributor Author

LJTian commented May 20, 2025

/hold

@k8s-ci-robot k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label May 20, 2025
@LJTian
fix QF1012: Use fmt.Fprintf(x, ...) instead of x.Write(fmt.Sprintf(...))
Loading
Loading status checks…
b0d09dc
@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels May 20, 2025
@LJTian
fix QF1001: could apply De Morgan's law
Loading
Loading status checks…
9e49d45
@medyagh medyagh changed the title Start repairing (currently submitting PR is just to facilitate discussion and view the current situation. When it needs to be merged, the title name will be changed and the administrator will be notified) WIP: Lint May 20, 2025
@k8s-ci-robot k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 20, 2025
@medyagh
Copy link
Member

medyagh commented May 20, 2025

Thank you @LJTian for starting this PR, I renamed it to WIP, you can rename the PR title (remove the WIP once you feel ready to be reviewed)

medyagh
medyagh requested changes May 20, 2025
View reviewed changes
@medyagh medyagh self-requested a review May 20, 2025 06:22
@LJTian
fix QF1003: could use tagged switch
Loading
Loading status checks…
a48f94a
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 20, 2025
LJTian added 4 commits May 20, 2025 07:23
@LJTian
fix weakCond: suspicious ; nil check may not be enough, check for len…
Loading
Loading status checks…
6a320d6 
… (gocritic)
@LJTian
fix docStub: silencing go lint doc-comment warnings is unadvised
Loading
Loading status checks…
59a51b4
@LJTian
fix builtinShadow: shadowing of predeclared identifier: error
Loading
Loading status checks…
07fdbb5
@LJTian
fix importShadow: shadow of imported package
Loading
Loading status checks…
c60ad81
@k8s-ci-robot k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 21, 2025
@LJTian
Copy link
Contributor Author

LJTian commented May 21, 2025 •
edited
Loading

Should I consider splitting this PR into multiple PRs? Create a PR for something simple and unambiguous and submit it first. Create a separate PR for larger changes?

@LJTian
fix nestingReduce: invert if cond, replace body with , move old body …
Loading
Loading status checks…
a1af98d 
…after the statement
LJTian added 2 commits May 21, 2025 07:21
@LJTian
useless-break: useless break in case clause (revive)
Loading
Loading status checks…
ece4796
@LJTian
Clear the redundant content in golangci.yaml file
Loading
Loading status checks…
e63bc48
@LJTian LJTian changed the title WIP: Lint fix lint issue May 23, 2025
@k8s-ci-robot k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 23, 2025
@medyagh
Copy link
Member

medyagh commented May 23, 2025

/ok-to-test

@k8s-ci-robot k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels May 23, 2025
@minikube-pr-bot
Copy link

kvm2 driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20804) |
+----------------+----------+---------------------+
| minikube start | 52.3s    | 52.5s               |
| enable ingress | 15.2s    | 15.4s               |
+----------------+----------+---------------------+

Times for minikube start: 53.5s 53.2s 51.3s 52.0s 51.6s
Times for minikube (PR 20804) start: 50.1s 52.9s 54.4s 53.6s 51.7s

Times for minikube ingress: 15.6s 15.0s 15.0s 15.0s 15.5s
Times for minikube (PR 20804) ingress: 15.0s 15.5s 15.1s 15.6s 16.0s

docker driver with docker runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20804) |
+----------------+----------+---------------------+
| minikube start | 23.8s    | 25.0s               |
| enable ingress | 13.0s    | 13.1s               |
+----------------+----------+---------------------+

Times for minikube start: 22.8s 21.9s 25.6s 26.1s 22.4s
Times for minikube (PR 20804) start: 25.8s 26.1s 25.8s 24.4s 22.7s

Times for minikube ingress: 12.8s 12.8s 12.8s 13.3s 13.3s
Times for minikube (PR 20804) ingress: 12.3s 13.3s 12.8s 13.3s 13.8s

docker driver with containerd runtime

+----------------+----------+---------------------+
|    COMMAND     | MINIKUBE | MINIKUBE (PR 20804) |
+----------------+----------+---------------------+
| minikube start | 22.8s    | 22.2s               |
| enable ingress | 34.2s    | 35.6s               |
+----------------+----------+---------------------+

Times for minikube (PR 20804) start: 23.3s 24.4s 21.1s 20.7s 21.5s
Times for minikube start: 24.1s 21.2s 22.9s 24.9s 21.1s

Times for minikube ingress: 30.3s 39.3s 23.8s 38.8s 38.8s
Times for minikube (PR 20804) ingress: 38.8s 38.8s 22.8s 38.8s 38.8s

medyagh
medyagh approved these changes May 24, 2025
View reviewed changes
Copy link
Member

@medyagh medyagh left a comment

Choose a reason for hiding this comment

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

excellent work @LJTian thank you for fixing this up ! I look forward to see more contributions from you! one thing that I noiced the docker-env and podman-env files share a lot of code, that is opportunity for more refactor to make them reuse the code instead of duplocation...

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: LJTian, medyagh

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:

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

@k8s-ci-robot k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 24, 2025
@medyagh medyagh changed the title fix lint issue build: address all lint issues by v2 May 24, 2025
@medyagh medyagh mentioned this pull request May 24, 2025
Open
@medyagh medyagh merged commit 3cf1e63 into kubernetes:master May 24, 2025
32 of 40 checks passed
@LJTian LJTian deleted the golangci-lint-v2 branch May 26, 2025 10:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prezha prezha

@medyagh medyagh

@ComradeProgrammer ComradeProgrammer

@spowelljr spowelljr

Assignees
No one assigned
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. ok-to-test Indicates a non-member PR verified by an org member that is safe to test. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Improve Lint and remove excludes
6 participants
@LJTian @k8s-ci-robot @minikube-bot @medyagh @minikube-pr-bot @prezha