fix: Update version to 1.5.9, correct update - #54
Merged
Conversation
service notification to use toast, and resolve localization parameter ordering.
Contributor
Reviewer's guide (collapsed on small PRs)Reviewer's GuideUpdates the bot to version 1.5.9 and fixes the update notification flow by switching a remaining chat-based update message to a localized toast notification and correcting localization argument ordering for toast messages, alongside updating changelog and release metadata. Sequence diagram for updated bot update notification flow using toast and corrected localizationsequenceDiagram
actor Broadcaster
participant GiveawayManager
participant CPHAdapter as CPHAdapter
participant UpdateService as UpdateService
participant Loc as Loc
Broadcaster->>GiveawayManager: ProcessTrigger(adapter)
GiveawayManager->>CPHAdapter: ShowToastNotification(Loc.Get(ToastTitle), Loc.Get(Update_Checking))
GiveawayManager->>UpdateService: CheckForUpdatesAsync(adapter, Version, true)
rect rgb(235, 245, 255)
UpdateService->>CPHAdapter: LogInfo or LogDebug
alt update available and saved
UpdateService->>Loc: Get(Update_Downloaded, null, remoteTag, fileName)
Loc-->>UpdateService: localizedMessage
UpdateService->>CPHAdapter: ShowToastNotification(Loc.Get(ToastTitle), localizedMessage)
else up to date and notifyIfUpToDate
UpdateService->>Loc: Get(Update_UpToDate, null, currentVersion)
Loc-->>UpdateService: localizedMessage
UpdateService->>CPHAdapter: ShowToastNotification(Loc.Get(ToastTitle), localizedMessage)
end
end
Updated class diagram for update notification and localization flowclassDiagram
class GiveawayManager {
<<static>> string Version
+bool ProcessTrigger(CPHAdapter adapter)
}
class UpdateService {
+static Task CheckForUpdatesAsync(CPHAdapter adapter, string currentVersion, bool notifyIfUpToDate)
}
class Loc {
+static string Get(string key, string profileName, params object[] args)
}
class CPHAdapter {
+void ShowToastNotification(string title, string message)
+void LogInfo(string message)
+void LogDebug(string message)
}
class Messenger {
+void SendBroadcast(CPHAdapter adapter, string message, string platform)
}
GiveawayManager --> UpdateService : uses
GiveawayManager --> CPHAdapter : uses
GiveawayManager --> Messenger : previously used for update notice
GiveawayManager --> Loc : uses for localized toast text
UpdateService --> CPHAdapter : uses for toast and logging
UpdateService --> Loc : uses for localized toast text
note for GiveawayManager "Version updated to 1_5_9 and update check now uses toast notifications instead of chat broadcast"
note for UpdateService "Localization arguments to Loc.Get now include explicit null profileName and correct ordering for toast messages"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- The new
Loc.Getcalls that passnullas theprofileNamemake the call sites a bit harder to read; consider introducing a small helper (e.g.,Loc.GetGlobal(key, params object[] args)) or using named arguments to avoid magicnullvalues scattered through the code. - The repeated use of the
"ToastTitle"localization key would be easier to maintain if it were centralized as a constant or static property rather than hard-coded in multiple call sites.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `Loc.Get` calls that pass `null` as the `profileName` make the call sites a bit harder to read; consider introducing a small helper (e.g., `Loc.GetGlobal(key, params object[] args)`) or using named arguments to avoid magic `null` values scattered through the code.
- The repeated use of the `"ToastTitle"` localization key would be easier to maintain if it were centralized as a constant or static property rather than hard-coded in multiple call sites.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
service notification to use toast, and resolve localization parameter ordering.
name: Pull Request
about: Propose changes to the bot
title: "[TYPE] Short description of change"
labels: ""
assignees: ""
Description
Type of change
Compatibility Check
new(), norecord, nousing var, etc.).editorconfigHow Has This Been Tested?
_tests/TestRunner.cs!giveaway system testChecklist
Summary by Sourcery
Bump the bot version to 1.5.9 and correct update and localization behavior for toast notifications.
Bug Fixes:
Documentation: