Skip to content

Conversation

@seanlaii
Copy link
Contributor

@seanlaii seanlaii commented Aug 29, 2025

Why are these changes needed?

golangci-lint v1 is no longer maintained and has reached end-of-life. To ensure code robustness and continue following Go best practices, we need to upgrade to golangci-lint v2.

This PR focuses on the configuration migration to unblock the upgrade

  1. Migrate golangci-lint to v2.7.2 format - Update the configuration file to be compatible with golangci-lint v2 syntax and structure.
  2. Temporarily disable problematic linters/formatters - Some linters and formatters produce new lint errors under v2. These are disabled in this PR to unblock the migration:
  • errorlint
  • gosec
  • govet
  • noctx
  • revive
  • staticcheck
  • testifylint
  • errcheck
  • gci

Follow-up work - The lint errors surfaced by these disabled linters will be addressed in subsequent PRs. These follow-up tasks are well-scoped and can be picked up by other contributors to parallelize the effort. The tasks are created in #4008.

Key changes include

Separation of Linters and Formatters: gofmt, goimports, and gci have been moved from the linters.enable list to the new top-level formatters.enable section, reflecting their primary role. Their settings are now under formatters.settings.
Merge of the Linters: staticcheck, stylecheck, and gosimple are merged into one linter, staticcheck.

Crucially, all custom configurations (gocyclo, gci, goimports) and specific path exclusions have been preserved and migrated to the new v2.0 schema. This upgrade ensures we leverage the latest improvements and features of golangci-lint while maintaining our established code quality standards.

For more details on the changes in v2.0, please refer to the official migration guide: https://golangci-lint.run/product/migration-guide/

Related issue number

Part of #4008

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

@seanlaii seanlaii changed the title Upgrade golangci-lint to v2.4.0 and adjust linting configurations [Chore] Upgrade golangci-lint to v2.4.0 and adjust linting configurations Aug 29, 2025
@seanlaii seanlaii force-pushed the golangci-v2 branch 2 times, most recently from 07b155b to d2a85d9 Compare August 29, 2025 02:54
Copy link
Contributor Author

@seanlaii seanlaii left a comment

Choose a reason for hiding this comment

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

Provide the details of the change to the golangci.yml.
Migration guide: https://golangci-lint.run/docs/product/migration-guide/

- unused
- wastedassign
- testifylint
disable-all: true
Copy link
Contributor Author

@seanlaii seanlaii Aug 29, 2025

Choose a reason for hiding this comment

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

Change disable-all to

linters:
  default: none

according to https://golangci-lint.run/docs/product/migration-guide/#lintersdisable-all.

- predeclared
- revive
- staticcheck
- typecheck
Copy link
Contributor Author

@seanlaii seanlaii Aug 29, 2025

Choose a reason for hiding this comment

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

typecheck is now enabled by default and cannot be disabled: https://golangci-lint.run/docs/product/migration-guide/#typecheck

require-explanation: true
require-specific: true
revive:
ignore-generated-header: true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

According to https://golangci-lint.run/docs/product/migration-guide/#linters-settingsreviveignore-generated-header, change it to:

linters:
  exclusions:
    generated: strict

.PHONY: lint
lint: golangci-lint fmt vet fumpt imports ## Run the linter.
# exclude the SA1019 check which checks the usage of deprecated fields.
test -s $(GOLANGCI_LINT) || ($(GOLANGCI_LINT) run --timeout=3m --exclude='SA1019' --no-config --allow-parallel-runners)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

--exclude is removed according to https://golangci-lint.run/docs/product/migration-guide/#command-line-flags; therefore, I add it in the golangci.yml.

for dir in $dirs_to_lint; do
pushd "$dir"
# exclude the SA1019 check which checks the usage of deprecated fields.
golangci-lint run --fix --exclude-files _generated.go --exclude='SA1019' --timeout 10m0s
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added it via linters.exclusions.paths in golangci.yml.

@dentiny
Copy link
Contributor

dentiny commented Dec 14, 2025

Hi @seanlaii , I'm wondering if there's any updates on this PR? I think it's required for golang version bump up, thanks!

@seanlaii
Copy link
Contributor Author

seanlaii commented Dec 14, 2025

Hi @seanlaii , I'm wondering if there's any updates on this PR? I think it's required for golang version bump up, thanks!

Got it. Thanks for the info! Will work on it today.

@seanlaii seanlaii marked this pull request as ready for review December 14, 2025 19:08
@seanlaii
Copy link
Contributor Author

Hi @Future-Outlier @rueian , please help take a look at this PR when you have a chance to unblock the golang version bump and the lint/format issue fix. Thanks!

@seanlaii seanlaii changed the title [Chore] Upgrade golangci-lint to v2.4.0 and adjust linting configurations [Chore] Upgrade golangci-lint to v2.7.2 and adjust linting configurations Dec 14, 2025
@Future-Outlier
Copy link
Member

Hi @Future-Outlier @rueian , please help take a look at this PR when you have a chance to unblock the golang version bump and the lint/format issue fix. Thanks!

thank you for pinging me, will make this PR this week's priority.

Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

Thank you!

Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

cc @win5923 to help if you have time, thank you!

Copy link
Collaborator

@win5923 win5923 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 your hard work!

lint: golangci-lint fmt vet ## Run the linter.
# exclude the SA1019 check which checks the usage of deprecated fields.
test -s $(GOLANGCI_LINT) || ($(GOLANGCI_LINT) run --timeout=3m --exclude='SA1019' --no-config --allow-parallel-runners)
test -s $(GOLANGCI_LINT) || ($(GOLANGCI_LINT) run --timeout=3m --allow-parallel-runners)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure if there’s a specific reason for using --no-config?
cc @rueian

Copy link
Member

@Future-Outlier Future-Outlier left a comment

Choose a reason for hiding this comment

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

cc @rueian to merge, thank you!

@rueian rueian merged commit e35feba into ray-project:master Dec 20, 2025
27 checks passed
@seanlaii seanlaii deleted the golangci-v2 branch December 20, 2025 00:51
@github-project-automation github-project-automation bot moved this from In Progress to Done in @Future-Outlier's kuberay project Dec 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants