-
Notifications
You must be signed in to change notification settings - Fork 1
feat: check for tool updates when printing current version #25
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
base: main
Are you sure you want to change the base?
Conversation
dido18
left a comment
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.
I would display a notification about a new version being available before every command, not only when running the version command.
The update check should be non-blocking — if an error occurs during the check, a general warning should be shown without preventing the command from running.
Co-authored-by: Per Tillisch <accounts@perglass.com>
.licenses/arduino-flasher-cli/go/github.com/arduino/go-paths-helper.dep.yml
Show resolved
Hide resolved
| type: go | ||
| summary: Package yaml implements YAML support for the Go language. | ||
| homepage: https://pkg.go.dev/gopkg.in/yaml.v3 | ||
| license: other |
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.
This is strange; the license isn't discovered, but the CI passes anyway. We should verify that everithing works as expected
| "github.com/arduino/arduino-flasher-cli/updater" | ||
| ) | ||
|
|
||
| type FlasherRelease struct { |
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.
This struct can be moved inline in the function; It is not needed anywhere else.
| color.YellowString(i18n.Tr("A new release of Arduino Flasher CLI is available:")), | ||
| color.CyanString(currentVersion.String()), | ||
| color.CyanString(latestVersion.String()), | ||
| color.YellowString("https://www.arduino.cc/en/software/#flasher-tool")) |
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.
Colored output is fancy, but I think we can omit it for now.
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.
I think we should keep them, but yeah, that stuff should be moved to the Cobra function
| color.CyanString(currentVersion.String()), | ||
| color.CyanString(latestVersion.String()), | ||
| color.YellowString("https://www.arduino.cc/en/software/#flasher-tool")) | ||
| feedback.Print(msg) |
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.
I would remove the feedback print from here and move into the main.go logic.
latest, err := checkForUpdate(current semver.Version)
if err != nil{
log.Warn(...)
}else{
feedback.Print("A new release of Arduino Flasher CLI is available:"....)
}
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.
For warning, we should use feedback.Warn
Motivation
Users should know if a new version of the tool is available.
Change description
A message is printed if a new version of the tool is available when checking the current version:
Additional Notes
Reviewer checklist
main.