Skip to content

Navigation Menu

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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
wants to merge 10 commits into
base: main
Choose a base branch
Loading
from

Conversation

stephenfin
Copy link
Contributor

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).

@github-actions github-actions bot added edit:networking This PR updates networking code edit:openstack This PR updates common OpenStack code edit:image This PR updates image code edit:orchestration This PR updates orchestration code edit:db This PR updates db code edit:loadbalancer This PR updates loadbalancer code edit:compute This PR updates compute code edit:keymanager This PR updates keymanager code edit:utils This PR updates utils code edit:messaging This PR updates messaging code edit:containerinfra This PR updates containerinfra code edit:objectstorage This PR updates objectstorage code edit:workflow This PR updates workflow code edit:sharedfilesystems This PR updates sharedfilesystems code edit:baremetal This PR updates baremetal code edit:gophercloud This PR updates common Gophercloud code edit:blockstorage This PR updates blockstorage code edit:identity This PR updates identity code edit:container This PR updates container code edit:baremetalintrospection This PR updates baremetalintrospection code edit:testinfra This PR updates testing infrastructure code semver:patch No API change labels May 16, 2025
@coveralls
Copy link

coveralls commented May 16, 2025

Coverage Status

coverage: 63.831% (+0.003%) from 63.828%
when pulling b685092 on stephenfin:golangci-lint-v2
into cf6f660 on gophercloud:main.

@stephenfin stephenfin requested a review from kayrus May 19, 2025 09:40
Copy link
Contributor

@kayrus kayrus left a 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

openstack/objectstorage/v1/objects/requests.go Outdated Show resolved Hide resolved
openstack/networking/v2/extensions/subnetpools/results.go Outdated Show resolved Hide resolved
openstack/compute/v2/services/results.go Outdated Show resolved Hide resolved
openstack/compute/v2/hypervisors/results.go Outdated Show resolved Hide resolved
openstack/compute/v2/hypervisors/results.go Outdated Show resolved Hide resolved
openstack/compute/v2/hypervisors/results.go Outdated Show resolved Hide resolved
openstack/compute/v2/hypervisors/results.go Outdated Show resolved Hide resolved
openstack/baremetalintrospection/noauth/requests.go Outdated Show resolved Hide resolved
openstack/baremetal/noauth/requests.go Outdated Show resolved Hide resolved
@pierreprinetti
Copy link
Member

don't mind me, just asking: are we sure those linters are worth the effort?

@kayrus
Copy link
Contributor

kayrus commented May 20, 2025

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>
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
Morty Proxy This is a proxified and sanitized view of the page, visit original site.