-
Notifications
You must be signed in to change notification settings - Fork 58
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
Show update notifications when a new version of kit is available #418
Conversation
Print a message before running commands if we find a newer version of Kit is available on GitHub. If the current version of Kit doesn't look to be a regular release, or if any error occurs while checking, we don't print anything (but do print some debug logs).
Add flag to version subcommand disable update notifications: kit version --show-update-notifications=false Settings is 'saved' by touching a file in $KITOPS_HOME Note, flag is attached to the 'version' subcommand as it seems there is no convenient way to specify it on the root command; it cannot be handled in PersistentPreRun and handling it in the root command's RunE breaks the help text for 'kit' (specifically, the 'usage' field)
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 it possible to print the new release note after the output of the command? Is that a better experience?
HarnessSubpath = "harness" | ||
HarnessProcessFile = "process.pid" | ||
HarnessLogFile = "harness.log" | ||
UpdateNotificationsConfigFilename = "disable-update-notifications" |
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.
Should we just have a kitconfig.json
where this is just a key?
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's a much bigger task :)
If we have a kitconfig.json, I'd expect to be able to almost configure everything there (e.g. something like viper). What that looks like is pretty nebulous.
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.
let's file an issue for it
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.
Filed #419
pkg/lib/update/update.go
Outdated
client := &http.Client{ | ||
Timeout: 1 * time.Second, | ||
} | ||
resp, err := client.Get(releaseUrl) | ||
if err != nil { | ||
output.Debugf("Failed to check for updates: %s", err) | ||
return | ||
} | ||
defer resp.Body.Close() | ||
|
||
respBody, err := io.ReadAll(resp.Body) | ||
if err != nil { | ||
output.Debugf("Failed to read GitHub response body: %s", err) | ||
return | ||
} | ||
info := &ghReleaseInfo{} |
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.
The code for retrieving the release info could be refactored into a function for clarity.
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.
Done
We could potentially put it in |
Description
Show an update notification in the CLI if a newer version is available on GitHub:
Notifications can be disabled by running
kit version --show-update-notifications=false
(use=true
to re-enable).Errors in checking for an update are delegated to debug-level logs and not shown to the user by default.
To test this PR, you will need to build kit with at least
-ldflags="-s -w -X kitops/pkg/lib/constants.Version=${version}"
Note: I had to put the enable/disable flag under the version subcommand, as trying to include it in the root command would either not run it or would break the default help text.
Linked issues
Closes #365