-
Notifications
You must be signed in to change notification settings - Fork 14
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
CI: Add gofmt check in lint #263
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Saurabh Kumar Singh <[email protected]>
Would it make more sense to include this in the golangci configuration that we use with |
Yes, you are correct. I raised the same question in yesterday's Arrow community meeting. I will incorporate the changes as per the review. |
@zeroshade I made changes by adding a hook, which you can see here. This raises the question: do I need to commit the changed files after running pre-commit cmd locally? |
That's the way it would work, yes. I'm not sure if we'd want it to automatically commit the changed files. |
Signed-off-by: Saurabh Kumar Singh <[email protected]>
Yes you were right I need to commit, and also, instead of hook, I chose to go ahead with |
Can we use https://github.com/golangci/golangci-lint ? We don't need to commit changes by pre-commit automatically. Developers should do it after they checked. |
Fixes: #58
What changes are included in this PR?
Added the
gofmt
check in the pre-commit-config.yml, which identifies files that needgofmt
linting and make proper linting.Are these changes tested?
Yes, I tested it on my forked repository example here