chore: don't message if Rover is already up to date #342
Conversation
EverlastingBugstopper
requested review from
JakeDawkins and
lrlna
as code owners
|
@EverlastingBugstopper Hmm, I agree for the automatic checks, I didn't think of that. This is also used by |
|
that sure does sound like a better approach! |
EverlastingBugstopper
force-pushed
the
avery/remove-rover-update-msg
branch
2 times, most recently
from
5c04dac
to
3ac24de
Compare
|
ok @JakeDawkins this is ready for re-review |
src/utils/version.rs
Outdated
| if is_foreground || last_checked_time.is_none() { | ||
| do_update_check(&mut checked, is_foreground)?; | ||
| } else if let Some(last_checked_time) = last_checked_time { | ||
| let time_since_check = current_time.duration_since(last_checked_time)?.as_secs(); | ||
| tracing::debug!( | ||
| "Time since last update check: {:?}h", | ||
| time_since_check / ONE_HOUR | ||
| ); | ||
|
|
||
| if force || time_since_check > ONE_DAY { | ||
| do_update_check(&mut checked)?; | ||
| } else { | ||
| tracing::debug!( | ||
| "No need to check for updates. Automatic checks happen once per day" | ||
| ); | ||
| } | ||
| } | ||
| // we haven't checked for updates before -- check now :) | ||
| None => { | ||
| do_update_check(&mut checked)?; | ||
| if time_since_check > ONE_DAY { | ||
| do_update_check(&mut checked, is_foreground)?; | ||
| } else { | ||
| tracing::debug!("No need to check for updates. Automatic checks happen once per day"); |
There was a problem hiding this comment.
i restructured this to try to make it a bit more readable - let me know if you think it has the opposite effect!
EverlastingBugstopper
force-pushed
the
avery/remove-rover-update-msg
branch
from
3ac24de
to
3611af3
Compare
src/utils/version.rs
Outdated
| /// If `force` is not passed, we check for updates every day at most | ||
| pub fn check_for_update(config: config::Config, force: bool) -> Result<()> { | ||
| /// If `is_foreground` is not passed, we check for updates every day at most | ||
| pub fn check_for_update(config: config::Config, is_foreground: bool) -> Result<()> { |
There was a problem hiding this comment.
I don't love the is_foreground name. It sounds less obvious what's going on than force imho. But I won't block on that at all :)
There was a problem hiding this comment.
mmm - i'll switch it back.
EverlastingBugstopper
force-pushed
the
avery/remove-rover-update-msg
branch
from
3611af3
to
7958660
Compare
cc @JakeDawkins on this one - felt a bit weird to have a message telling you you're up to date. I feel like it's OK to just remain silent in that case. What do you think?