-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
fix(report): close the file #4842
Conversation
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.
LGTM.
Left 1 idea.
pkg/report/writer.go
Outdated
output := os.Stdout | ||
if option.Output != "" { | ||
f, err := os.Create(option.Output) | ||
if err != nil { | ||
return xerrors.Errorf("failed to create a file: %w", err) | ||
} | ||
output = f | ||
defer f.Close() | ||
} |
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.
what if we move this logic into separate function and use it for aws(https://github.com/aquasecurity/trivy/pull/4842/files#diff-62eef55c4c7ff14ad4fdbadd93c6348d55e1ba37b204b43e6e054a51dab293d6R62-R70) and k8s(https://github.com/aquasecurity/trivy/pull/4842/files#diff-3cf1939a3a0885c6af6d90b0f4d07617761f8d332db3523421f81e4ad373d123R99-R106)
pkg/commands/app.go
Outdated
@@ -1218,13 +1207,12 @@ func showVersion(cacheDir, outputFormat, version string, outputWriter io.Writer) | |||
|
|||
switch outputFormat { | |||
case "json": | |||
b, _ := json.Marshal(VersionInfo{ | |||
_ = json.NewEncoder(w).Encode(VersionInfo{ |
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.
error is ignored here, was it done intentionally ?
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.
That is not the code I added, and I have no idea why the error was ignored initially. But it looks like we should handle it. Fixed in 1417f18.
* fix(report): close the file * refactor: add the format type * fix: return errors in version printing * fix: lint issues * fix: do not fail on bogus cache dir --------- Co-authored-by: DmitriyLewen <dmitriy.lewen@smartforce.io>
Description
Close the output file. Also, this PR treats the output value as string so that we can add support for #4451.
Also, this PR defines
type Format string
.Related issues
Checklist