-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: add copyloopvar linter #4382
Conversation
I don't understand why you created a new PR instead of updating the old one #4182 |
In order for a pull request adding a linter to be reviewed, the linter and the PR must follow some requirements.
Pull Request Description
Linter
The Linter Tests Inside Golangci-lint
|
Thank you for your swift response. I opened a new PR because I let the before PR sit for a long time. |
I also don't understand why you created a new repo instead of renaming the old one. I think you should archive the old repo:
|
I archived the old one. |
I opened a PR karamaru-alpha/copyloopvar#1 and you seem to not see it. You should look at your GitHub notifications and maybe your emails because we will have a problem if someone reports an issue or opens a PR on your repository. I trying to help you, and I will help you, but I cannot do it well if you don't watch your repository. |
@ldez |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
Signed-off-by: karamaru-alpha <mrnk3078@gmail.com>
bbefb60
to
40cd9ea
Compare
I made a suggestion to improve the report message: karamaru-alpha/copyloopvar#2 Before:
After:
|
@ldez go run ./cmd/golangci-lint/ run --no-config --disable-all --enable=copyloopvar ./test/testdata/copyloopvar.go
|
FYI to run the test of your linter inside golangci-lint: T=copyloopvar.go make test_linters https://golangci-lint.run/contributing/new-linters/#how-to-add-a-public-linter-to-golangci-lint |
Thanks for letting me know! T=copyloopvar.go make test_linters
|
It was just advice for you, our CI runs all tests 😉 |
I'd like to add https://github.com/karamaru-alpha/copyloopvar.
This linter detects places where loop variables are copied.
For Go 1.21(set GOEXPERIMENT=loopvar) and Go 1.22~, it is unnecessary to copy loop variables because these variables have per-iteration scope instead of per-loop scope.
cf. Fixing For Loops in Go 1.22