-
Notifications
You must be signed in to change notification settings - Fork 29
fix: improve confusing update messaging #1682
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
Conversation
cmd/root.go
Outdated
| wf, _, _ := utils.GetWorkflow() | ||
|
|
||
| // If a workflow file is present and specifies that the speakeasyVersion is "latest", don't display update notifications as it will be automatically updated when the command is run. | ||
| if wf != nil && wf.SpeakeasyVersion.String() == "latest" { |
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'd want to render the update available when a workflow file is present but when we're (for instance) running any non-workflow command? I.e. I think it's only workflow run which has weird output
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.
Ah fair play - it's only run that does the auto upgrade
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 fixed up 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.
@ThomasRooney bump on the review here ☝🏻
Think my scoping down is the right solution
ThomasRooney
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.
need to scope the change more IMO
c8d293f to
b0808a0
Compare
The CLI confusingly displays two conflicting messages - this is because in the logic for displaying the message "Running with latest Speakeasy version" which is based on looking at the workflow.yaml
speakeasyVersion: latestvalue, we do not actually check that we are on latest before rendering it (you can see from the subsequent screenshot that we do eventually autoupgrade)