Skip to content

Conversation

@fscnick
Copy link
Collaborator

@fscnick fscnick commented Jan 28, 2025

Why are these changes needed?

Make the err as local variable in if-statement because they are used in the scope only.

Related issue number

Checks

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

Signed-off-by: fscnick <fscnick.dev@gmail.com>
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.

would you mind checking whether is there any lint rule to check this?

@kevin85421 kevin85421 merged commit 2b31c30 into ray-project:master Jan 28, 2025
24 checks passed
owenowenisme pushed a commit to owenowenisme/kuberay that referenced this pull request Jan 29, 2025
Signed-off-by: fscnick <fscnick.dev@gmail.com>
@fscnick
Copy link
Collaborator Author

fscnick commented Jan 30, 2025

would you mind checking whether is there any lint rule to check this?

It seems not able to do that under golangci-lint currently.

There was a rule ifshort but deprecated now. An alternative errifinline is not merged. golangci/golangci-lint#4471 (comment)

if trying to enable ifshort in the config. it prompts the error message as follow:
image

@fscnick fscnick deleted the chore-make-err-as-local-variable branch January 31, 2025 08:58
Ygnas pushed a commit to Ygnas/kuberay that referenced this pull request Mar 20, 2025
Signed-off-by: fscnick <fscnick.dev@gmail.com>
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.

2 participants