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

🌱 Update golangci-lint (v1.46.2 -> v1.50.0), remove deprecated linters #1342

Merged
merged 1 commit into from
Oct 10, 2022

Conversation

oscr
Copy link
Contributor

@oscr oscr commented Sep 13, 2022

What this PR does / why we need it:

  • Updates golangci-lint version (v2.46.2 -> v1.49.0)
  • Removed deprecated linters and configuration
    • deadcode
    • ifshort
    • structcheck
    • varcheck
  • Fixes or adds //nolints.

/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 Sep 13, 2022
@netlify
Copy link

netlify bot commented Sep 13, 2022

Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!

Name Link
🔨 Latest commit 940af7d
🔍 Latest deploy log https://app.netlify.com/sites/kubernetes-sigs-cluster-api-openstack/deploys/6342992f053cc90007b155ea
😎 Deploy Preview https://deploy-preview-1342--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 cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Sep 13, 2022
@seanschneeweiss
Copy link
Contributor

Can you adopt the title and replace v2.46.2 with v1.46.2.

Maybe we can also enable some linters as they did in CAPI: kubernetes-sigs/cluster-api#7208

@tobiasgiese
Copy link
Member

tobiasgiese commented Sep 19, 2022

/retitle 🌱 Update golangci-lint (v2.46.2 -> v2.49.0), remove deprecated linters

Edit: oops 😅

@k8s-ci-robot k8s-ci-robot changed the title 🌱 Update golangci-lint (v2.46.2 -> v1.49.0), remove deprecated linters, 🌱 Update golangci-lint (v2.46.2 -> v2.49.0), remove deprecated linters Sep 19, 2022
@tobiasgiese
Copy link
Member

/retitle 🌱 Update golangci-lint (v1.46.2 -> v1.49.0), remove deprecated linters

@k8s-ci-robot k8s-ci-robot changed the title 🌱 Update golangci-lint (v2.46.2 -> v2.49.0), remove deprecated linters 🌱 Update golangci-lint (v1.46.2 -> v1.49.0), remove deprecated linters Sep 19, 2022
@oscr
Copy link
Contributor Author

oscr commented Sep 19, 2022

It's not easy with all the different version numbers 😓 Thanks for the fix @tobiasgiese 🚀

@jichenjc
Copy link
Contributor

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 22, 2022
@oscr oscr force-pushed the update-golangci-lint branch from cc404d9 to 940af7d Compare October 9, 2022 09:49
@k8s-ci-robot k8s-ci-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 9, 2022
@oscr oscr changed the title 🌱 Update golangci-lint (v1.46.2 -> v1.49.0), remove deprecated linters 🌱 Update golangci-lint (v1.46.2 -> v1.50.0), remove deprecated linters Oct 9, 2022
@oscr
Copy link
Contributor Author

oscr commented Oct 9, 2022

Just bumped the golangci-lint version since a new version has been released since the pr was created.

@mdbooth
Copy link
Contributor

mdbooth commented Oct 10, 2022

/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: mdbooth, oscr

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 Oct 10, 2022
@jichenjc
Copy link
Contributor

/lgtm

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

tobiasgiese commented Oct 10, 2022

Maybe we can also enable some linters as they did in CAPI: kubernetes-sigs/cluster-api#7208

@oscr Can you say something about the open suggestion from @seanschneeweiss?

@mdbooth
Copy link
Contributor

mdbooth commented Oct 10, 2022

Maybe we can also enable some linters as they did in CAPI: kubernetes-sigs/cluster-api#7208

@oscr Can you say something about the open suggestion from @seanschneeweiss?

@tobiasgiese I saw that but thought they could be added separately. Would you like me to remove the approve until it's answered?

@tobiasgiese
Copy link
Member

Would you like me to remove the approve until it's answered?

It's okay for me to keep the approve label. If @oscr wants to remove the hold and create a follow-up PR it's totally fine for me :)

@oscr
Copy link
Contributor Author

oscr commented Oct 10, 2022

@tobiasgiese @mdbooth Thank you for pointing out the question. To be honest I just missed it. I suggest enabling additional linters in a separate pr. Some of them give a lot of findings. However I would be glad to do it.

@tobiasgiese
Copy link
Member

Thanks for the clarification @oscr 🙂
/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 Oct 10, 2022
@k8s-ci-robot k8s-ci-robot merged commit 81f39b8 into kubernetes-sigs:main Oct 10, 2022
@oscr oscr deleted the update-golangci-lint branch October 17, 2022 09:01
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/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants