-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
ci: golangci-lint bump, fixes, and cleanups #3615
Merged
Merged
Conversation
This file contains 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
Fix these two (reported by golangci-lint v1.61.0): resctrl/utils.go:180:15: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) return nil, fmt.Errorf(noContainerNameError) ^ resctrl/utils.go:214:14: SA1006: printf-style function with dynamic format string and no further arguments should use print-style function instead (staticcheck) return "", fmt.Errorf(noPidsPassedError) ^ Signed-off-by: Kir Kolyshkin <[email protected]>
Fix the following errcheck warning (reported by golangci-lint v1.61.0): container/libcontainer/handler.go:741:13: Error return value of `fmt.Sscanf` is not checked (errcheck) fmt.Sscanf(fs[4], "%X:%X", &rx, &tx) ^ While at it, simplify instantiation of the adjacent variables. Signed-off-by: Kir Kolyshkin <[email protected]>
As reported by golangci-lint v1.61.0: internal/storage/bigquery/client/client.go:231:21: printf: non-constant format string in call to fmt.Errorf (govet) return fmt.Errorf(errstr) ^ Signed-off-by: Kir Kolyshkin <[email protected]>
Fix a few warnings like this one (currently masked by config): utils/cpuload/netlink/netlink.go:50:14: Error return value of `binary.Write` is not checked (errcheck) binary.Write(w, Endian, m.GenHeader) ^ Signed-off-by: Kir Kolyshkin <[email protected]>
With commit 300242c, my intention was to use whatever version of golangci-lint is available locally, and if not (which is usually the case in CI), install the version recommended by the Makefile. The code was changed since, and currently has two issues wrt how golangci-lint is installed: 1. golangci-lint prints version without v prefix (e.g. "... version 1.23.45 ..."), while grep looks for v1.23.45. As a result, golangci-lint is *always* reinstalled. 2. If a newer golangci-lint version is installed locally, running "make lint" will downgrade it. This is not what most developers would expect. Let's fix both. A version in the Makefile is now minimally required version, and no attempt to downgrade is made. Fixes: ea3afbc ("Build changes for golang 1.18") Signed-off-by: Kir Kolyshkin <[email protected]>
While at it, declutter .golangci.yml by removing most exceptions. Signed-off-by: Kir Kolyshkin <[email protected]>
Oh well, can I please have a free a CI run here? 😆 |
This is not used and was not used for quite some time, so can be safely removed. If there's a need to check for capitalized error messages, we can setup stylecheck linter with enabled ST1005 (see https://staticcheck.io/docs/configuration/options/#checks). I ran this checker just for fun, and it gives 20+ warnings for ST1005 only, so I'm not enabling it here. For completeness, here's how a warning from the removed script looks like: > Incorrect error format in file ./client/client.go: return fmt.Errorf("Status code is not OK: %v (%s)", resp.StatusCode, resp.Status) And here's the same warning from staticcheck: > client/client.go:214:10: ST1005: error strings should not be capitalized (stylecheck) > return fmt.Errorf("Status code is not OK: %v (%s)", resp.StatusCode, resp.Status) > ^ To me, it makes sense to not capitalize error messages (and fix those that are capitalized), but it's up to maintainers to decide. Signed-off-by: Kir Kolyshkin <[email protected]>
Merged
iwankgb
approved these changes
Oct 26, 2024
/ok-to-test |
@bobbypage ptal |
/test pull-cadvisor-e2e |
/approve |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Please see individual commits for details.