Skip to content

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

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

linting added to go.yml ref:2468 #51

wants to merge 5 commits into from

Conversation

KaushikiAnand
Copy link

  • 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.

@KaushikiAnand KaushikiAnand requested review from a team as code owners March 5, 2025 07:17
Copy link

@johanbrandhorst johanbrandhorst left a 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 ./...

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?

Copy link
Author

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.

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.

Copy link
Author

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.

Copy link

@johanbrandhorst johanbrandhorst left a 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

@abhijeetviswa
Copy link

Superseded by #51

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.

None yet

3 participants