-
Notifications
You must be signed in to change notification settings - Fork 311
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
Support for "okteto doctor" command #719
Conversation
pkg/cmd/doctor/run.go
Outdated
} | ||
fmt.Fprintf(fileSummary, "version=%s\n", config.VersionString) | ||
fmt.Fprintf(fileSummary, "os=%s\n", runtime.GOOS) | ||
fileSummary.Sync() |
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 return value of fileSummary.Sync
is not checked (from errcheck
)
pkg/cmd/doctor/run.go
Outdated
return "", err | ||
} | ||
fmt.Fprint(fileRemoteLog, remoteLogs) | ||
fileRemoteLog.Sync() |
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 return value of fileRemoteLog.Sync
is not checked (from errcheck
)
Codecov Report
@@ Coverage Diff @@
## master #719 +/- ##
==========================================
- Coverage 31.98% 31.65% -0.33%
==========================================
Files 58 59 +1
Lines 4549 4596 +47
==========================================
Hits 1455 1455
- Misses 2999 3046 +47
Partials 95 95
Continue to review full report at Codecov.
|
pkg/cmd/doctor/run.go
Outdated
if err != nil { | ||
log.Yellow("Failed to query remote syncthing logs: %s", err) | ||
} | ||
if remoteLogsPath != "" { |
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.
is this check necessary if there's no error?
pkg/cmd/doctor/run.go
Outdated
return fmt.Errorf("couldn't create archive, please try again") | ||
} | ||
|
||
log.Information("Your doctor file was generated at %s", archiveName) |
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.
don't call log.Information
outside of the cmd
package, so we can control UI elements there. I think you can return the file and do the notification there.
pkg/cmd/doctor/run.go
Outdated
return "", err | ||
} | ||
fmt.Fprintf(fileSummary, "version=%s\n", config.VersionString) | ||
fmt.Fprintf(fileSummary, "os=%s\n", runtime.GOOS) |
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.
add the arch as well
pkg/cmd/doctor/run.go
Outdated
if err := fileSummary.Sync(); err != nil { | ||
return "", err | ||
} | ||
fileSummary.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.
put it as defer fileSummary.Close()
in line 82 instead.
pkg/cmd/doctor/run.go
Outdated
if err != nil { | ||
return "", err | ||
} | ||
fmt.Fprintf(fileSummary, "version=%s\n", config.VersionString) |
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.
we should probably check the error of Fprintf. Maybe create the string first and just write once?
Co-Authored-By: Ramiro Berrelleza <ramiro@okteto.com>
Co-Authored-By: Ramiro Berrelleza <ramiro@okteto.com>
Co-Authored-By: Ramiro Berrelleza <ramiro@okteto.com>
Closes #663