Skip to content

ci: switch to golangci-lint v2 #4692

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

Merged
merged 6 commits into from
Mar 31, 2025
Merged

Conversation

kolyshkin
Copy link
Contributor

@kolyshkin kolyshkin commented Mar 24, 2025

Switch to golangci v2.0.

The new configuration file was initially generated by golangci-lint
migrate, when tweaked to minimize and simplify.

golangci-lint v2 switches to a new version of staticcheck which shows
much more warnings. Some of them were fixed by a few previous commits,
and the rest of them are disabled.

In particular, ST1005 had to be disabled (an attempt to fix it was made
in #3857 but it wasn't merged).

> notify_socket.go:44:24: ST1016: methods on the same type should have the same receiver name (seen 1x "n", 5x "s") (staticcheck)
> func (s *notifySocket) Close() error {
>                        ^

As reported by staticcheck from golangci-lint v2.0.0

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
> libcontainer/rootfs_linux.go:1255:13: QF1004: could use strings.ReplaceAll instead (staticcheck)
> 	keyPath := strings.Replace(key, ".", "/", -1)

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
Like these:

> libcontainer/criu_linux.go:959:3: QF1001: could apply De Morgan's law (staticcheck)
> 		!(req.GetType() == criurpc.CriuReqType_FEATURE_CHECK ||
> 		^
> libcontainer/rootfs_linux.go:360:19: QF1001: could apply De Morgan's law (staticcheck)
> 	if err == nil || !(errors.Is(err, unix.EPERM) || errors.Is(err, unix.EBUSY)) {
> 	                 ^

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
I was pretty sure we have a linter for these but apparently we did not.

> libcontainer/capabilities/capabilities.go:108:1: ST1020: comment on exported method ApplyCaps should be of the form "ApplyCaps ..." (staticcheck)
> // Apply sets all the capabilities for the current process in the config.
> ^
>
>
> types/events.go:15:1: ST1021: comment on exported type Stats should be of the form "Stats ..." (with optional leading article) (staticcheck)
> // stats is the runc specific stats structure for stability when encoding and decoding stats.
> ^

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
> libcontainer/intelrdt/cmt.go:5:1: ST1020: comment on exported function IsCMTEnabled should be of the form "IsCMTEnabled ..." (staticcheck)
> // Check if Intel RDT/CMT is enabled.
> ^
> libcontainer/intelrdt/intelrdt.go:419:1: ST1020: comment on exported function IsCATEnabled should be of the form "IsCATEnabled ..." (staticcheck)
> // Check if Intel RDT/CAT is enabled
> ^
> libcontainer/intelrdt/intelrdt.go:425:1: ST1020: comment on exported function IsMBAEnabled should be of the form "IsMBAEnabled ..." (staticcheck)
> // Check if Intel RDT/MBA is enabled
> ^
> libcontainer/intelrdt/intelrdt.go:446:1: ST1020: comment on exported method Apply should be of the form "Apply ..." (staticcheck)
> // Applies Intel RDT configuration to the process with the specified pid
> ^
> libcontainer/intelrdt/intelrdt.go:481:1: ST1020: comment on exported method Destroy should be of the form "Destroy ..." (staticcheck)
> // Destroys the Intel RDT container-specific 'container_id' group
> ^
> libcontainer/intelrdt/intelrdt.go:497:1: ST1020: comment on exported method GetPath should be of the form "GetPath ..." (staticcheck)
> // Returns Intel RDT path to save in a state file and to be able to
> ^
> libcontainer/intelrdt/intelrdt.go:506:1: ST1020: comment on exported method GetStats should be of the form "GetStats ..." (staticcheck)
> // Returns statistics for Intel RDT
> ^
> libcontainer/intelrdt/mbm.go:6:1: ST1020: comment on exported function IsMBMEnabled should be of the form "IsMBMEnabled ..." (staticcheck)
> // Check if Intel RDT/MBM is enabled.
> ^
> 8 issues:
> * staticcheck: 8

While at it, add missing periods.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
The new configuration file was initially generated by golangci-lint
migrate, when tweaked to minimize and simplify.

golangci-lint v2 switches to a new version of staticcheck which shows
much more warnings. Some of them were fixed by a few previous commits,
and the rest of them are disabled.

In particular, ST1005 had to be disabled (an attempt to fix it was made
in opencontainers#3857 but it wasn't
merged).

Also, golangci-extra was modified to include ALL staticcheck linters.

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@kolyshkin kolyshkin requested a review from thaJeztah March 30, 2025 07:11
Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

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

LGTM

@thaJeztah thaJeztah merged commit e8a97ba into opencontainers:main Mar 31, 2025
34 checks passed
@lifubang lifubang added the backport/1.3-done A PR in main branch which has been backported to release-1.3 label Apr 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/ci backport/1.3-done A PR in main branch which has been backported to release-1.3
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants