Bump golangci-lint to the latest version (v2)#4377
Bump golangci-lint to the latest version (v2)#4377k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Conversation
k8s-ci-robot
added
kind/cleanup
k8s-ci-robot
added
cncf-cla: yes
erikgb
force-pushed
the
golangci-lint-v2
branch
2 times, most recently
from
ab9410d to
5ced3e5
Compare
| // Set up plaintext server. | ||
| lis, err := net.Listen("tcp", fmt.Sprintf("0.0.0.0:%d", config.HTTPPort)) | ||
| plc := net.ListenConfig{} | ||
| lis, err := plc.Listen(ctx, "tcp", fmt.Sprintf("0.0.0.0:%d", config.HTTPPort)) |
There was a problem hiding this comment.
out of my curiosity, was this a bug or a linter complain?
There was a problem hiding this comment.
Everything in this PR is done to fix new linter complains. In this case, ListenConfig is introduced to add a Listen function accepting a context.
|
@erikgb overall lgtm, do you mind running a conformance test against one of the implementations with these changes just to be sure no behavior will be broken? Thanks |
|
no, they are not. Because the majority of changes are made on conformance package, this is my bigger concern. You can install Istio, Cilium, Envoy Gateway or any other implementation and run it locally it with something like: but will you need to set the right test to run (eg. I was testing |
|
@rikatz, I am not sure if I am able to run these tests on my workstation, as it is set up in a wierd way (company laptop). Are you able to help? The lint fixes should be pretty safe, as I have done similar changes in multiple projects without issues. |
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
erikgb
force-pushed
the
golangci-lint-v2
branch
from
5ced3e5 to
95f24c6
Compare
|
I have now tried to run these conformance tests, both on the upstream default branch and from this branch, but with no success. Errors are the same on both branches, so I am still fairly certain that the changes proposed in this PR don't break anything. My problems running the tests are certainly caused by something wrong in my local setup. I've tried both Traefik and Envoy Gateway. After spending dozens of hours on debugging this, I would really appreciate some help in running these tests, if it's required to get this PR merged. 🙏 And if running these tests is required to get changes merged, I frankly don't understand why none of the tests are run as part of CI. 🙅 |
|
/retest |
|
Tested locally the GRPC tests, after rebuilding the echo-basic image and using it on my tests, works fine. Also all the migrations / fixes seems correct. /lgtm |
k8s-ci-robot
added
the
lgtm
|
thanks @erikgb ! |
|
Thanks @erikgb! /approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: erikgb, rikatz, robscott, snorwin The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
Signed-off-by: Erik Godding Boye <egboye@gmail.com>
5 participants
What type of PR is this?
/kind cleanup
What this PR does / why we need it:
In #4376 it is required to bump the minimum Go version to Go 1.25, and golangci-lint v1 does not support Go 1.25.
I used
golangci-lint migrateto migrate the golangci-lint configuration and manually ported the comments from the existing configuration, as the migration tool removes comments. Some new errors were introduced by v2, but were easy to fix.Which issue(s) this PR fixes:
Fixes #
Does this PR introduce a user-facing change?: