-
Notifications
You must be signed in to change notification settings - Fork 553
Bump golangci-lint to v2 and fix new lint issues #3385
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
Open
stephenfin
wants to merge
10
commits into
gophercloud:main
Choose a base branch
from
stephenfin:golangci-lint-v2
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
a99c5c5
to
417f785
Compare
kayrus
reviewed
May 20, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the PR, see my comments
don't mind me, just asking: are we sure those linters are worth the effort? |
the ones that modify error messages? partly... |
Done by bumping the version in the Makefile and running the 'golangci-lint migrate' command via podman as we do in the Makefile. podman run -t --rm -v $PWD:/app -w /app \ golangci/golangci-lint:v2.1.6 \ golangci-lint migrate This yields a number of news error from staticcheck, listed below, which we temporarily ignore pending a fix. S1038: should use fmt.Fprintf instead of fmt.Fprint(fmt.Sprintf(...)) ST1005: error strings should not be capitalized ST1005: error strings should not end with punctuation or newlines ST1017: don't use Yoda conditions ST1019: package ".*" is being imported more than once ST1019(related information): other import of ".*" ST1023: should omit type .* from declaration; it will be inferred from the right-hand side QF1011: could omit type .* from declaration; it will be inferred from the right-hand side QF1001: could apply De Morgan's law QF1003: could use tagged switch on .* QF1004: could use strings.ReplaceAll instead QF1008: could remove embedded field ".*" from selector Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
This looks complicated, but it's just a whole lot of sed invocations. Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
More sed, along with some manual fixups. sed -i 's/fmt.Errorf("\([A-Z]\)/fmt.Errorf("\L\1/' \ $(ag 'fmt.Errorf\("[A-Z]' -l) sed -i 's/fmt.Errorf("\(.*\)\.")/fmt.Errorf("\1")/' \ $(ag 'fmt.Errorf\(' -l) sed -i 's/errors.New("\([A-Z]\)/errors.New("\L\1/' \ $(ag 'errors.New\("[A-Z]' -l) sed -i 's/errors.New("\(.*\)\.")/errors.New("\1")/' \ $(ag 'errors.New\(' -l) (yes, that could have a one-liner but $complexity) Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
Signed-off-by: Stephen Finucane <stephenfin@redhat.com>
417f785
to
b685092
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
edit:baremetal
This PR updates baremetal code
edit:baremetalintrospection
This PR updates baremetalintrospection code
edit:blockstorage
This PR updates blockstorage code
edit:compute
This PR updates compute code
edit:container
This PR updates container code
edit:containerinfra
This PR updates containerinfra code
edit:db
This PR updates db code
edit:gophercloud
This PR updates common Gophercloud code
edit:identity
This PR updates identity code
edit:image
This PR updates image code
edit:keymanager
This PR updates keymanager code
edit:loadbalancer
This PR updates loadbalancer code
edit:messaging
This PR updates messaging code
edit:networking
This PR updates networking code
edit:objectstorage
This PR updates objectstorage code
edit:openstack
This PR updates common OpenStack code
edit:orchestration
This PR updates orchestration code
edit:sharedfilesystems
This PR updates sharedfilesystems code
edit:testinfra
This PR updates testing infrastructure code
edit:utils
This PR updates utils code
edit:workflow
This PR updates workflow code
semver:patch
No API change
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a large but relatively trivial PR that, per $subject, bumps our version of golangci-lint to the latest version and addresses any conflicts introduced. The only user-facing change is that some errors messages have changed (lowercase first characters and stripped full stops).