Bug/1555 auto update check uses hours component of timespan rather than totalhours #1556
Conversation
WalkthroughThe pull request updates the Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant AppUpdateService
participant Logger
Caller->>AppUpdateService: ShouldCheckForUpdate()
alt ForceUpdateCheckEnvVar is set
AppUpdateService->>Logger: Log "Forced update check triggered"
AppUpdateService-->>Caller: Return update check result (true)
else
AppUpdateService->>AppUpdateService: Calculate timeSinceLastCheck (TotalHours)
alt timeSinceLastCheck.TotalHours < -1
AppUpdateService->>Logger: Log "Last update check is in the future"
AppUpdateService-->>Caller: Return false
else if timeSinceLastCheck.TotalHours < 8
AppUpdateService->>Logger: Log "Skipping update: last check too recent"
AppUpdateService-->>Caller: Return false
else
AppUpdateService->>Logger: Log "Triggering update check based on last check time"
AppUpdateService-->>Caller: Return true
end
end
Assessment against linked issues
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
backend/FwLite/FwLiteMaui/Platforms/Windows/AppUpdateService.cs(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Analyze (csharp)
- GitHub Check: Build FW Lite and run tests
🔇 Additional comments (3)
backend/FwLite/FwLiteMaui/Platforms/Windows/AppUpdateService.cs (3)
212-217: Good switch to TotalHours for checking future timestampsGreat improvement changing from
HourstoTotalHoursfor detecting if the last check time is in the future. This ensures accuracy when the difference spans across day boundaries.
218-222: Using TotalHours and threshold adjustmentGood change to use
TotalHoursinstead ofHoursfor checking the time since the last update check. This makes the check independent of the time of day when the code is executed.Note that the threshold has been reduced from 20 hours to 8 hours, which means updates will be checked more frequently. This is likely intentional but worth confirming.
Was the reduction in threshold from 20 to 8 hours intentional? This will increase the frequency of update checks.
224-225: Improved logging for update check decisionsThe added log message provides valuable information about why an update check is being performed, which will help with debugging and monitoring.
hahn-kev
deleted the
bug/1555-auto-update-check-uses-hours-component-of-timespan-rather-than-totalhours
branch
2 participants
Resolves #1555
ShouldCheckForUpdate is no longer unintentionaly dependant on the time of day the code runs.
I tried to add a test, but couldn't figure out how to properly reference the Maui project or the Windows specific code or how to run the tests. 🙃
You can see my effort here: #1557
Summary by CodeRabbit