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

etcd: modify declaring empty slices #14479

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

demoManito
Copy link
Contributor

@demoManito demoManito commented Sep 16, 2022

declare an empty slice to var s []int replace s :=[]int{}, https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

declare an empty slice to var s []int replace  s :=[]int{}, https://github.com/golang/go/wiki/CodeReviewComments#declaring-empty-slices

Signed-off-by: demoManito <1430482733@qq.com>
@ahrtr
Copy link
Member

ahrtr commented Sep 16, 2022

Please fix the test failures.

Signed-off-by: demoManito <1430482733@qq.com>
@serathius
Copy link
Member

Please consider introducing a static analysis that will prevent future regressions. I expect something already exists in golangci, just needs to be enabled in https://github.com/etcd-io/etcd/blob/main/.golangci.yaml

@ahrtr
Copy link
Member

ahrtr commented Sep 16, 2022

Please consider introducing a static analysis that will prevent future regressions. I expect something already exists in golangci, just needs to be enabled in https://github.com/etcd-io/etcd/blob/main/.golangci.yaml

Yes, let's do this in a separate PR.

@demoManito Please fix the test failures.

@demoManito
Copy link
Contributor Author

demoManito commented Sep 16, 2022

Please consider introducing a static analysis that will prevent future regressions. I expect something already exists in golangci, just needs to be enabled in main/.golangci.yaml

Yes, let's do this in a separate PR.

@demoManito Please fix the test failures.

@ahrtr test success

@cenkalti
Copy link
Member

I couldn't find any linter for this task in https://golangci-lint.run/usage/linters/

Copy link
Member

@ahrtr ahrtr left a comment

Choose a reason for hiding this comment

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

LGTM

Thank you @demoManito

cc @spzala @serathius @mitake to double check

@ahrtr
Copy link
Member

ahrtr commented Sep 19, 2022

The change looks safe to me, so merging it

@ahrtr
Copy link
Member

ahrtr commented Sep 19, 2022

I couldn't find any linter for this task in https://golangci-lint.run/usage/linters/

Thanks @cenkalti for the feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants