-
Notifications
You must be signed in to change notification settings - Fork 356
feat: introduce golangci-lint #794
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
Conversation
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
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.
Pull Request Overview
This PR introduces golangci-lint to the project by adding linter configuration and addressing various code issues identified by the linter. The primary purpose is to improve code quality and consistency across the Go codebase by implementing automated static analysis.
- Adds golangci-lint configuration file and GitHub Actions workflow for automated linting
- Fixes numerous error handling issues by adding
//nolint:errcheckdirectives where appropriate - Removes unused variables and functions, and simplifies code constructs
- Corrects import aliases and function references for consistency
Reviewed Changes
Copilot reviewed 36 out of 36 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| go/.golangci.yaml | Adds golangci-lint configuration with standard linters enabled |
| .github/workflows/ci.yaml | Adds GitHub Actions job for running golangci-lint |
| go/Makefile | Updates golangci-lint version to v2.4.0 and import path |
| Multiple Go files | Adds //nolint:errcheck directives and removes unused code |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| return &mockObject{ | ||
| TypeMeta: m.TypeMeta, | ||
| ObjectMeta: *m.ObjectMeta.DeepCopy(), | ||
| ObjectMeta: *m.DeepCopy(), |
Copilot
AI
Aug 21, 2025
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.
The method call should be m.ObjectMeta.DeepCopy() instead of m.DeepCopy(). The current code is calling DeepCopy on the entire mockObject instead of just the ObjectMeta field.
| ObjectMeta: *m.DeepCopy(), | |
| ObjectMeta: *m.ObjectMeta.DeepCopy(), |
| Authorizer: authorizer, | ||
| Authenticator: authenticator, | ||
| }) | ||
| if err != nil { |
Copilot
AI
Aug 21, 2025
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.
The error handling is checking for an error from the NewHTTPServer call, but the error variable err was not declared or assigned from that function call. The NewHTTPServer call on line 438-444 should assign its return value to include the error.
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io> Signed-off-by: Fabian Gonzalez <fabian.gonzalez@solo.io>
Signed-off-by: Eitan Yarmush <eitan.yarmush@solo.io>
No description provided.