-
Notifications
You must be signed in to change notification settings - Fork 625
Description
Summary
This issue serves as a central tracker for the task of upgrading our project's golangci-lint version from an outdated v1.64.8
to a modern v2.4.0
.
Upgrading our primary linter is crucial for several reasons:
- Enhanced Code Quality: Access to newer, more intelligent linters and security checks.
- Improved Go Compatibility: Ensures full compatibility with the latest Go versions (e.g., Go 1.24) and language features.
- Bug Fixes & Performance: Leverages numerous bug fixes and performance improvements from the golangci-lint team.
- Dependency Hygiene: Keeps our developer toolchain modern and secure.
Proposed Plans
Option 1: A direct upgrade introduces a significant number of new linting errors across our components. To manage this without disrupting the CI on our main branch, I propose the following phased approach:
- Fix Errors Incrementally: We will address the new linting errors through a series of smaller, targeted Pull Requests. Each PR will fix a specific category of lint errors (as detailed in the task list below). These PRs will be merged into master.
- Evaluate Rules & Allow Disabling: It is important to note that not all identified issues must be fixed. As we tackle each category, we should evaluate the utility of the specific linting rule. If a rule is found to be not valuable, too noisy, or the required changes are too extensive for the benefit they provide, we can open a discussion to disable that rule in our
.golangci.yml
configuration instead of implementing the fixes. - Final Version Bump: Once all linting errors are resolved and main is clean, a final PR will be merged to officially update the golangci-lint version in .pre-commit-config.yaml and .golangci.yml.
This strategy ensures our CI remains green throughout the process and makes the review process for each change more manageable.
Option 2: Start by upgrading the config and temporarily disabling the linters that report errors. Then, re-enable them in separate PRs and fix the issues one by one.
Action Items / Linting Errors to Fix
Component: ray-operator
(Total: 226 issues)
-
staticcheck (208 issues):
-
ST1003 (94): Fix incorrectly formatted style in error strings.
-
QF1008 (92): Simplify code by omitting embedded fields from selectors.
-
ST1001 (7): Fix dot-imports that are not in a test package.
-
ST1005 (6): Fix incorrectly formatted error strings.
-
QF1004 (4): Correct the usage of time.Sub to time.Since.
-
QF1003 (2): Simplify if statements.
-
ST1023 (1): Fix incorrectly formatted variable/constant comments.
-
ST1019 (2): Fix issues related to importing the same package multiple times.
-
testifylint (8 issues):
-
len (5): Fix incorrect usage of assert.Equal for checking length.
-
empty (2): Fix incorrect usage of assert.Equal for checking if a value is empty.
-
equal-values (1): Fix incorrect usage of assert.Equal for comparing values.
-
noctx (4 issues):
-
os/exec.Command: Replace os/exec.Command with os/exec.CommandContext to allow for cancellation.
-
goimports (4 issues):
-
Fix Go import order and formatting.
-
revive (2 issues):
-
var-naming: Fix variable names that do not conform to the naming convention.
Components: kubectl-plugin, apiserver, apiserversdk
(Total: 171 issues)
-
staticcheck (124 issues):
-
QF1008 (60): Simplify code by omitting embedded fields from selectors.
-
ST1005 (40): Fix incorrectly formatted error strings.
-
ST1003 (19): Fix incorrectly formatted style in error strings.
-
ST1001 (2): Fix dot-imports that are not in a test package.
-
ST1023 (1): Fix incorrectly formatted variable/constant comments.
-
ST1019 (2): Fix issues related to importing the same package multiple times.
-
revive (23 issues):
-
var-naming (15): Fix variable names that do not conform to the naming convention.
-
unexported-return (5): Fix exported functions returning unexported types.
-
unused-parameter (3): Remove unused function parameters.
-
noctx (11 issues):
-
os/exec.Command (9): Replace os/exec.Command with os/exec.CommandContext.
-
net.Listen (2): Replace net.Listen with (*net.ListenConfig).Listen.
-
testifylint (8 issues): (@seanlaii Fix testifylint & gci lint issues in apiserver/kubectl-plugin #4014)
-
empty (7): Fix incorrect usage of assert.Equal for checking if a value is empty.
-
len (1): Fix incorrect usage of assert.Equal for checking length.
-
errorlint (2 issues):
-
Replace == or != with errors.Is for comparing wrapped errors.
-
gci (1 issue): (@seanlaii Fix testifylint & gci lint issues in apiserver/kubectl-plugin #4014)
-
Fix Go import order and formatting.
-
gosec (1 issue):
-
G114: Address potential security issue with net/http cookie security.
-
govet (1 issue):
-
fieldalignment: Fix struct field alignment for performance.
Final Step
- Upgrade PR: Once all the above tasks are completed, create and merge the final PR that updates the golangci-lint version. ([Chore] Upgrade golangci-lint to v2.4.0 and adjust linting configurations #4007)