-
Notifications
You must be signed in to change notification settings - Fork 12
linting added to go.yml ref:2468 #51
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
base: main
Are you sure you want to change the base?
Conversation
KaushikiAnand
commented
Mar 5, 2025
- I have added linting to the go.yml workflow.
- The linting was done to ensure that the codebase is consistent and maintainable and helps in improving the code quality and reducing errors.
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.
Thanks, this looks better, one suggestion and one question
.PHONY: lint | ||
lint: | ||
staticcheck -checks=all,-U1000,-ST1023 ./... |
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.
Why is U1000 and ST1023 ignored?
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.
I ignored ST1023(validate.go:35:8: should omit type reflect.Value from declaration; it will be inferred from the right-hand side) because on removing the reflect.Value the make-gen-delta workflow was not getting successful built.
I ignored U1000 as it showed a variable being unused, so i didn’t have a proper idea about that variable so didn’t proceed in correcting it.
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.
I think both of those should be re-enabled. If the workflow is failing, that's telling us that it's running correctly. I would be happy to coach you in how to fix the warnings instead of ignoring them globally.
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.
yeah i am working upon it.
as soon as i am able to find a solution i will push my changes.
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.
Remove the linting exceptions please
Superseded by #51 |