Skip to content
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

Bump the golangci-lint version in the api server makefile #1342

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

z103cb
Copy link
Contributor

@z103cb z103cb commented Aug 17, 2023

Why are these changes needed?

The version of golangci-lint used in the KubeRay api server hangs when used with higher versions of golang (1.20+). This is related to this issue

Related issue number

Closes #1334

Checks

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

@z103cb z103cb changed the title Z103cb/issue1334 Bump the golangci-lint version in the api server makefile Aug 17, 2023
@z103cb
Copy link
Contributor Author

z103cb commented Aug 17, 2023

@blublinsky, @tedhtchang, @scarlet25151 and @anishasthana,
Can you please take a look ?

cc: @kevin85421

Copy link
Contributor

@blublinsky blublinsky left a comment

Choose a reason for hiding this comment

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

LGTM, hardcoding the version is a bit of concern

@@ -122,7 +122,7 @@ KIND ?= $(REPO_ROOT_BIN)/kind
KUSTOMIZE_VERSION ?= v3.8.7
GOFUMPT_VERSION ?= v0.3.1
GOIMPORTS_VERSION ?= latest
GOLANGCI_LINT_VERSION ?= v1.50.1
GOLANGCI_LINT_VERSION ?= v1.54.1
KIND_VERSION ?= v0.19.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have to put here a specific version? Can we just use the latest? Or figure out the version based on the go version? I am a bit concerned with the maintenance of updating the version constantly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. I think we should pin the version as it will give us consistent builds. The maintenance is small price to pay.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed that we should pin the verison

Copy link
Contributor

@anishasthana anishasthana left a comment

Choose a reason for hiding this comment

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

/lgtm

cc @kevin85421

Copy link
Member

@kevin85421 kevin85421 left a comment

Choose a reason for hiding this comment

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

LGTM

@kevin85421 kevin85421 merged commit 06ccd09 into ray-project:master Aug 17, 2023
22 checks passed
blublinsky pushed a commit to blublinsky/kuberay that referenced this pull request Aug 22, 2023
…t#1342)

Bump the golangci-lint version in the api server makefile
blublinsky pushed a commit to blublinsky/kuberay that referenced this pull request Aug 25, 2023
…t#1342)

Bump the golangci-lint version in the api server makefile
lowang-bh pushed a commit to lowang-bh/kuberay that referenced this pull request Sep 24, 2023
…t#1342)

Bump the golangci-lint version in the api server makefile
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.

[Bug] API server make file uses golangci-lint version, that hangs
4 participants