Fix restart not showing new version
#3686
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3686 +/- ##
==========================================
+ Coverage 88.85% 88.92% +0.07%
==========================================
Files 254 254
Lines 14269 14269
==========================================
+ Hits 12679 12689 +10
+ Misses 1590 1580 -10
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This looks good to me as is and as such I'm going to approve.
However, I think some of the logic can be improved.
write_replyis unnecessary since writing an empty reply is idempotent- The
config->update_prompt->is_time_to_show()check andconfig->update_prompt->populate()can be simplified with a single call topopulate_if_time_to_show().populate()displays the new version info every time the command is called, which is probably a bit too spammy.populate_if_time_to_show()will only display new version info everynotify_user_frequencywhich is inline with what the other commands do.
|
Hmm, I would rather avoid being too naggy. @Sploder12 WDYT? |
I agree, I'd want to avoid that too @ricab. |
|
Alright, let's wait for it then, thanks. |
There was a problem hiding this comment.
LGTM! Will wait for CI before merging.
BTW, if you use one of these keywords along with the issue number, the issue will be closed automatically when the PR gets merged.
|
Fixes #3672 |
Fix `restart` not showing new version
Partial fix for #3672, unable to reproduce
startbehavior.When running
restartwith a new version of Multipass available, no notification would be shown but the field is present in the gRPC reply.This PR makes it so the daemon properly populates that field for
restartand has the CLI respond to it.