Skip to content

[golangci-lint] Enable deprecated API warnings linter#4605

Merged
andrewsykim merged 1 commit intoray-project:masterfrom
mboersma:linter-enable-SA1019
Apr 15, 2026
Merged

[golangci-lint] Enable deprecated API warnings linter#4605
andrewsykim merged 1 commit intoray-project:masterfrom
mboersma:linter-enable-SA1019

Conversation

@mboersma
Copy link
Copy Markdown
Contributor

Why are these changes needed?

kuberay has had SA1019 disabled in the golangci-lint config, which led to some deprecated API calls remaining. This enables the linter in general, but carves out exceptions for kuberay's own deprecated fields.

Related issue number

Checks

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

@mboersma mboersma changed the title [golangci-lint] Enable linter deprecated API warnings [golangci-lint] Enable deprecated API warnings linter Mar 16, 2026
Copy link
Copy Markdown
Member

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

LGTM, could you also update the comments in these files. Thanks!

kuberay/scripts/lint.sh

Lines 9 to 11 in 476c902

# Exclude the SA1019 check which checks the usage of deprecated fields
# via the linters.settings.staticcheck.checks configuration in .golangci.yaml.
golangci-lint run --fix --timeout 10m0s

# Exclude the SA1019 check which checks the usage of deprecated fields
# via the linters.settings.staticcheck.checks configuration in .golangci.yaml.
.PHONY: lint
lint: golangci-lint fmt vet ## Run the linter.
test -s $(GOLANGCI_LINT) || ($(GOLANGCI_LINT) run --timeout=3m --allow-parallel-runners)

# Exclude the SA1019 check which checks the usage of deprecated fields
# via the linters.settings.staticcheck.checks configuration in .golangci.yaml.
.PHONY: lint
lint: golangci-lint fmt vet fumpt imports ## Run the linter.
test -s $(GOLANGCI_LINT) || ($(GOLANGCI_LINT) run --timeout=3m --allow-parallel-runners)

Comment thread .golangci.yml
Comment on lines +102 to +107
# NewSimpleClientset is deprecated in favor of NewClientset, but
# NewClientset requires a complete SMD schema for field management
# which breaks Update-based tests. TODO: migrate once SMD schemas
# are properly generated for all kuberay types.
- linters: [staticcheck]
text: "SA1019:.*NewSimpleClientset"
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you open a issue to track this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

See #4627.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@win5923 thanks for the review! I removed the outdated comments about SA1019 and opened an issue to track SMD schemas.

@mboersma
Copy link
Copy Markdown
Contributor Author

@win5923 I think this is ready to go if you have time to take another look. (See also #4634.)

Copy link
Copy Markdown
Member

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

LGTM! sorry for the delay, I was on vacation the past few weeks.

@mboersma
Copy link
Copy Markdown
Contributor Author

Thanks @win5923, no problem--I hope you had a great vacation.

What's needed now to get this merged? Should I ask another maintainer for a review?

@win5923
Copy link
Copy Markdown
Member

win5923 commented Apr 14, 2026

Yeah, I think Andrew or Rueian could help with a final review.

@mboersma
Copy link
Copy Markdown
Contributor Author

ping @andrewsykim @rueian

Copy link
Copy Markdown
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.

I didn't review this PR, but I think we can trust @win5923 's review

@andrewsykim andrewsykim merged commit 0d2892e into ray-project:master Apr 15, 2026
31 checks passed
@github-project-automation github-project-automation bot moved this from can be merged to Done in @Future-Outlier's kuberay project Apr 15, 2026
@mboersma mboersma deleted the linter-enable-SA1019 branch April 16, 2026 02:05
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.

4 participants