Skip to content
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

Merged
merged 4 commits into from
Jul 19, 2024

Conversation

amisevsk
Copy link
Contributor

Description

Show an update notification in the CLI if a newer version is available on GitHub:

Note: A new version of Kit is available! You are using Kit v0.2.2. The latest version is v0.3.0.
      To see a list of changes, visit https://github.com/jozu-ai/kitops/releases/tag/v0.3.0
      To disable this notification, use 'kit version --show-update-notifications=false'

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

@amisevsk amisevsk requested a review from gorkem July 18, 2024 21:13
amisevsk added 3 commits July 18, 2024 17:23
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)
@amisevsk amisevsk changed the title Update notif Show update notifications when a new version of kit is available Jul 18, 2024
Copy link
Contributor

@gorkem gorkem left a 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"
Copy link
Contributor

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?

Copy link
Contributor Author

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.

Copy link
Contributor

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Filed #419

pkg/cmd/version/version.go Show resolved Hide resolved
Comment on lines 68 to 83
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{}
Copy link
Contributor

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@amisevsk
Copy link
Contributor Author

Is it possible to print the new release note after the output of the command? Is that a better experience?

We could potentially put it in postRun, but in my experience most applications print update notifications at the top before showing you the output you want. We don't want to be intrusive.

@amisevsk amisevsk merged commit 1181c4a into jozu-ai:main Jul 19, 2024
3 checks passed
@amisevsk amisevsk deleted the update-notif branch July 19, 2024 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add notification for CLI users for new version
2 participants