-
Notifications
You must be signed in to change notification settings - Fork 398
Description
π Prerequisites
- I have searched the existing issues to avoid creating a duplicate
- By submitting this issue, you agree to follow our Code of Conduct
π Feature Summary
Add a new CI check for go code linting
β Problem Statement / Motivation
In the contribution guidelines, it is mentioned that we should run make lint for go changes. While this is a good thing to do, currently there are pre-existing failed lint checks.
Over time, this list will likely grow making it harder for developers trying to focus on their specific changes and linting issues. At best, someone will focus on their linting issues and resolve a few other straightforward linting errors. At worst, over time there will so many unrelated linting issues that developers will opt to avoid/ignore running make lint.
π‘ Proposed Solution
We should create a new GitHub Workflow to run a lint check on go code. It could be as simple as running the make lint command.
When implemented, it may make sense to resolve all (or as many) of the existing linting errors that exist. Based on the resolvability of some, assuming there is no way to do a "lint-ignore", we would decide if this should be a merge-blocking check or just an informative one.
π Alternatives Considered
- We could not have this action and have trust we all run
make lint, but I believe this is something that could potentially slip someone's mind. - We could resolve the linting errors on a if-a-developer-comes-across-it basis, but it could be overbearing for someone to work on their changes, resolve unrelated linting errors, and get feedback based on that.
π― Affected Service(s)
None
π Additional Context
Below are the current lint fails for main
cli/internal/cli/dashboard_darwin.go:46:12: Error return value of `fmt.Scanln` is not checked (errcheck)
fmt.Scanln() // wait for Enter Key
^
cli/cmd/kagent/main.go:75:28: Error return value of `invokeCmd.MarkFlagRequired` is not checked (errcheck)
invokeCmd.MarkFlagRequired("task")
^
controller/internal/httpserver/handlers/health.go:23:9: Error return value of `w.Write` is not checked (errcheck)
w.Write([]byte("OK"))
^
controller/internal/httpserver/handlers/helpers.go:36:9: Error return value of `w.Write` is not checked (errcheck)
w.Write(response)
^
controller/internal/httpserver/handlers/invoke.go:111:10: Error return value of `w.Write` is not checked (errcheck)
w.Write([]byte(fmt.Sprintf("event: %s\ndata: %s\n\n", event.Event, event.Data)))
^
controller/internal/httpserver/handlers/teams.go:140:31: Error return value of `kubeClientWrapper.AddInMemory` is not checked (errcheck)
kubeClientWrapper.AddInMemory(teamRequest)
^
controller/internal/httpserver/middleware_error.go:58:27: Error return value of `(*encoding/json.Encoder).Encode` is not checked (errcheck)
json.NewEncoder(w).Encode(map[string]string{"error": responseMessage})
^
cli/internal/cli/chat.go:162:5: var `thinkingVerbs` is unused (unused)
var thinkingVerbs = []string{"thinking", "processing", "mulling over", "pondering", "reflecting", "evaluating", "analyzing", "synthesizing", "interpreting", "inferring", "deducing", "reasoning", "evaluating", "synthesizing", "interpreting", "inferring", "deducing", "reasoning"}
^
cli/internal/cli/chat.go:164:6: func `getThinkingVerb` is unused (unused)
func getThinkingVerb() string {
^
controller/internal/httpserver/handlers/tools.go:83:6: func `convertMapToMCPToolConfig` is unused (unused)
func convertMapToMCPToolConfig(data map[string]v1alpha1.AnyType) (api.MCPToolConfig, error) {
^
controller/internal/httpserver/helpers.go:10:6: func `respondWithJSON` is unused (unused)
func respondWithJSON(w http.ResponseWriter, code int, payload interface{}) {
^
controller/internal/httpserver/helpers.go:22:6: func `respondWithError` is unused (unused)
func respondWithError(w http.ResponseWriter, code int, message string) {
^
controller/internal/httpserver/helpers.go:26:6: func `getUserID` is unused (unused)
func getUserID(r *http.Request) (string, error) {
^
cli/internal/cli/invoke.go:145:2: S1023: redundant `return` statement (gosimple)
return
^
controller/internal/autogen/autogen_reconciler.go:543:3: SA4006: this value of `existingToolServer` is never used (staticcheck)
existingToolServer, err = a.autogenClient.CreateToolServer(toolServer, common.GetGlobalUserID())
^
controller/internal/httpserver/handlers/sessions.go:123:2: SA4006: this value of `log` is never used (staticcheck)
log = log.WithValues("userID", userID)
^
controller/internal/httpserver/handlers/sessions.go:155:2: SA4006: this value of `log` is never used (staticcheck)
log = log.WithValues("userID", userID)
^
controller/internal/httpserver/handlers/sessions.go:231:2: SA4006: this value of `log` is never used (staticcheck)
log = log.WithValues("sessionID", sessionID)
^
controller/internal/httpserver/handlers/teams.go:74:6: SA4031: this nil check is never true (staticcheck)
if modelConfig == nil {
^
controller/internal/httpserver/handlers/teams.go:65:19: SA4031(related information): this is the value of modelConfig (staticcheck)
modelConfig := &v1alpha1.ModelConfig{}
^
make: *** [lint] Error 1
(I wasn't sure which type makes most sense for a CI-oriented issue, so sorry if this is the wrong type)
π Are you willing to contribute?
- I am willing to submit a PR for this feature
Metadata
Metadata
Assignees
Labels
Type
Projects
Status