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 go1.19 linting errors #1313

Merged

Conversation

tobiasgiese
Copy link
Member

@tobiasgiese tobiasgiese commented Jul 27, 2022

What this PR does / why we need it:

Currently we're facing the following issue:

test/e2e/shared/suite.go:27:2: SA1019: package io/ioutil is deprecated: As of Go 1.16, the same functionality is now provided by package io or package os, and those implementations should be preferred in new code. See the specific function documentation for details. (staticcheck)
	"io/ioutil"

xref

See go-critic/go-critic#1019

Further we have to make our linter happy, because the most recent test-infra image gcr.io/k8s-staging-test-infra/kubekins-e2e:v20220726-61b7bafca6-master is using go1.19-rc2 (see kubernetes/test-infra#26920)

❯ docker run -ti --entrypoint bash gcr.io/k8s-staging-test-infra/kubekins-e2e:v20220726-61b7bafca6-master
[...]
root@a4f4d8285e82:/workspace# go version
go version go1.19rc2 linux/amd64

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

  1. Please confirm that if this PR changes any image versions, then that's the sole change this PR makes.

TODOs:

  • squashed commits
  • if necessary:
    • includes documentation
    • adds unit tests

Tobias Giese tobias.giese@mercedes-benz.com, Mercedes-Benz Tech Innovation GmbH, legal info/Impressum

@netlify
Copy link

netlify bot commented Jul 27, 2022

βœ… Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
πŸ”¨ Latest commit 5edcac1
πŸ” Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/62e107a5729d300008162e5b
😎 Deploy Preview https://deploy-preview-1313--kubernetes-sigs-cluster-api-openstack.netlify.app
πŸ“± Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 27, 2022
@k8s-ci-robot k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2022
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/ioutil-deprecated branch from 5cda7ed to e0f3a57 Compare July 27, 2022 09:16
@k8s-ci-robot k8s-ci-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Jul 27, 2022
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/ioutil-deprecated branch from e0f3a57 to f6727e2 Compare July 27, 2022 09:24
@apricote
Copy link
Member

/lgtm

Thanks for taking care of all these little things the last weeks @tobiasgiese !

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2022
Signed-off-by: Tobias Giese <tobias.giese@mercedes-benz.com>
@tobiasgiese tobiasgiese force-pushed the tobiasgiese/ioutil-deprecated branch from f6727e2 to 5edcac1 Compare July 27, 2022 09:38
@k8s-ci-robot k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Jul 27, 2022
@tobiasgiese tobiasgiese changed the title πŸ› Remove deprecated ioutil and use os instead πŸ› Fix go1.19 linting errors Jul 27, 2022
@apricote
Copy link
Member

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 27, 2022
@tobiasgiese
Copy link
Member Author

/hold

We should discuss if want to fix these errors because of golangci-lint@v1.46.2 or if we want to downgrade to v1.44.0 (or similar).
See kubernetes-sigs/cluster-api#6737 (thx @chrischdi)

@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 Jul 27, 2022
Copy link
Contributor

@seanschneeweiss seanschneeweiss left a comment

Choose a reason for hiding this comment

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

/lgtm

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: seanschneeweiss, tobiasgiese

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 [seanschneeweiss,tobiasgiese]

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

@seanschneeweiss
Copy link
Contributor

πŸ‘ for fixing these errors.

Should we update golangci-lint to v1.47.2 in a following PR?

@tobiasgiese
Copy link
Member Author

Should we update golangci-lint to v1.47.2 in a following PR?

I'd suggest to not update golangci-lint to v1.47.2, see kubernetes-sigs/cluster-api#6737 (comment)

v1.47.1 does not work due to performance issues in the revive linter. v1.47.2 disables the problematic linters allowing that version of golangci-lint to be used on CAPI at the cost of some linter coverage.

@tobiasgiese
Copy link
Member Author

tobiasgiese commented Jul 27, 2022

But I think it's fair to fix these errors (i.e., merge this PR).
k/k is doing this as well (kubernetes/kubernetes#111254)

@tobiasgiese
Copy link
Member Author

/hold cancel

@k8s-ci-robot k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jul 27, 2022
@k8s-ci-robot k8s-ci-robot merged commit 96ee54a into kubernetes-sigs:main Jul 27, 2022
@tobiasgiese tobiasgiese deleted the tobiasgiese/ioutil-deprecated branch July 27, 2022 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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. lgtm "Looks good to me", indicates that a PR is ready to be merged. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants