Feature: Introduce golangci-lint #77
Conversation
|
@jyane I'm not super familiar with linting around Golang. Do you have any opinion or comments about this PR? |
|
I'm also not familar with the tools, and I don't have any preference. My 2 cents is that:
|
I completely agree. I also had concerns about enabling errcheck.
As you mentioned, it's a trade-off between the diffs needed to fix linter errors and improving the overall quality of the codebase. TBH, I don't have any particular preference for golangci-lint or individual linters (and their configurations) too; If there are any exemplary configurations from your company or other OSS projects, we might consider adopting those settings. |
I agree with it. I think it's would be better to have the linter.
We have the Go style guide but it doesn't limit us to use the other linter tool. And Overall, I agree with using Can you fix the CI failing? If it shows a lot of errors and takes time, I'll fix these lint errors in this PR if you want. |
Signed-off-by: Junya Okabe <86868255+Okabe-Junya@users.noreply.github.com>
8f902cd to
2472685
Compare
chore: fix linter error
|
Thank you, I fixed some CI errors (excluding the linter error fixes) (200d287, 9b20056), And I fixed some linter errors in 03c4a3e, but it seems there are still many errors – many of which are related to fieldalignment and shadow. I don’t have time to fix these immediately, but if you can wait a little longer – about 1~2 weeks – I can address them. Of course, if you decide to work with them sooner, it’s perfectly fine to merge this PR. In that case, there is a workaround to avoid CI failure by setting the exit code of golangci-lint to 0. FYI: https://golangci-lint.run/usage/configuration/#run-configuration Thanks! |
| // RawToken is the actual token in string representation. | ||
| RawToken string | ||
| // ValidAtLeastUntil holds the expiration time when the information is available. | ||
| // The token refreshers won't refresh token if this value exists and later than now. | ||
| // The default value will be `January 1, year 1, 00:00:00.000000000 UTC` and it is earlier than the possible time.Now(). | ||
| // The default value naturally means the token is expired. |
| @@ -52,12 +52,9 @@ type InspectionRunResult struct { | |||
|
|
|||
| // InspectionTaskServer manages tasks and provides apis to get task related information in JSON convertible type. | |||
| type InspectionTaskServer struct { | |||
| // rootTaskSet is the set of the all definitions in KHI. | |||
| State enum.RevisionState `json:"state"` | ||
|
|
||
| // DEPRECATED: This field is no longer used. Will be removed in near future. |
| // Common fields assigned by log entity | ||
| Timestamp time.Time `json:"ts"` | ||
| // ID is an actual unique ID of this log. This field must be unique. KHI uses `insertId`-`timestamp` for GCP log. | ||
| ID string `json:"id"` | ||
| // Display ID is a log ID directly visible to user. This field no need to be unique. KHI uses `insertId` for GCP log. | ||
| DisplayId string `json:"displayId"` | ||
| Body *binarychunk.BinaryReference `json:"body"` | ||
|
|
||
| // These fields are managed by each parsers | ||
| Type enum.LogType `json:"type"` | ||
| Timestamp time.Time `json:"ts"` |
| // records lease history of NEG id to ServiceNetworkEndpointGroup | ||
| NEGs *resourcelease.ResourceLeaseHistory[*resourcelease.K8sResourceLeaseHolder] | ||
|
|
||
| nodeNames map[string]struct{} |
| // Type of input field. Currently, only `text` or `popup_redirect` is the supported value. | ||
| Type string | ||
| // Description of this form. | ||
| Options map[string]string `json:"options"` |
| Subtractives []string | ||
| ValidationError string | ||
| // The default mode is additive mode. The empty filter means none should be selected. | ||
| // SubtractMode will match anything. The empty filter means anything should be selected, this field will be true when user specify @any in the field. | ||
| SubtractMode bool | ||
| SubtractMode bool |
| // Current Log | ||
| Log *log.LogEntity | ||
| // ResourceName field of the log | ||
| ResourceName string | ||
| // MethodName field of the log | ||
| MethodName string | ||
| // PrincipalEmail field of the log | ||
| PrincipalEmail string | ||
| // Kubernetes operation read from resource name and method name | ||
| Operation *model.KubernetesObjectOperation | ||
| // The request field of this log. This can be nil depending on the audit policy. | ||
| Request *structure.Reader | ||
| RequestType rtype.Type | ||
| // The response field of this log. This can be nil depending on the audit policy. | ||
| Response *structure.Reader | ||
| ResponseType rtype.Type | ||
| // Current resource body canged by this request. | ||
| ResourceBodyReader *structure.Reader | ||
| ResourceBodyYaml string | ||
| // The response code. | ||
| Code int | ||
|
|
|
Thank you for fixing these lint error. The CI seems to output better lint errors.
I feel
Please take your time. If you think the remaining part of lint errors should be fixed from my side, please feel free to assign this PR to me. Then I will work fixing the remaining part. |
|
Or how do you think to create another Github Action task to perform golangci-lint? |
What
make lint-gofrom go vet to golangci-lintBackground
When hacking on this codebase, I noticed that the (Go) linter run by pre-commit was failing like this:
In addition, to detect several potential bugs in the codebase, I want to introduce a more comprehensive linter (or linter runner), such as golangci-lint.
Results
execution result
Next steps