Conversation
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
TerryHowe
left a comment
There was a problem hiding this comment.
Normal logging, I would expect the messages to start with an uppercase character, but with structured logging, I'm not sure. It seems like most of the existing messages start with a lower case charactger.
mattfarina
left a comment
There was a problem hiding this comment.
Note: I was looking up if log messages should start with an uppercase or lowercase letter. There is no consistency here. Even in the Go standard library. Projects like Kubernetes are also inconsistent.
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
mattfarina
left a comment
There was a problem hiding this comment.
I noticed that errors are sometimes keyed as "err" and other times keyed as "error". Can we please have this be consistent with "error"?
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
I addressed this for you as well. |
pkg/chart/v2/util/dependencies.go
Outdated
| vv, err := cvals.Table(r.Name + "." + child) | ||
| if err != nil { | ||
| log.Printf("Warning: ImportValues missing table from chart %s: %v", r.Name, err) | ||
| slog.Warn("ImportValues missing table from chart", "chart", r.Name, "value", err) |
There was a problem hiding this comment.
Should it be "error" instead of "value"
pkg/engine/engine.go
Outdated
| if e.LintMode { | ||
| // Don't fail on missing required values when linting | ||
| log.Printf("[INFO] Missing required value: %s", warn) | ||
| slog.Warn("missing required value", "value", warn) |
There was a problem hiding this comment.
In this case the warn is a message that's shared with the template executor when the required thing is not present. So, the key name could be something like message.
pkg/engine/engine.go
Outdated
| if e.LintMode { | ||
| // Don't fail on missing required values when linting | ||
| log.Printf("[INFO] Missing required value: %s", warn) | ||
| slog.Warn("missing required values", "value", warn) |
There was a problem hiding this comment.
In this case the warn is a message that's shared with the template executor when the required thing is not present. So, the key name could be something like message.
pkg/engine/engine.go
Outdated
| if e.LintMode { | ||
| // Don't fail when linting | ||
| log.Printf("[INFO] Fail: %s", msg) | ||
| slog.Info("funcMap fail", "lintMode", msg) |
There was a problem hiding this comment.
message also makes sense as a key name here. That's what's being passed.
pkg/engine/lookup_func.go
Outdated
| resList, err := discoveryClient.ServerResourcesForGroupVersion(gvk.GroupVersion().String()) | ||
| if err != nil { | ||
| log.Printf("[ERROR] unable to retrieve resource list for: %s , error: %s", gvk.GroupVersion().String(), err) | ||
| slog.Error("unable to retrieve resource list", "list", gvk.GroupVersion().String(), "error", err) |
There was a problem hiding this comment.
Instead of list for the key, it should be GroupVersion.
Signed-off-by: Robert Sirchia <rsirchia@outlook.com>
I converted inline log to slog. This is my first pass there will be more when i jump into other sections.