refactor: simplify published_at parsing and fix Localization double-checked locking#174
Draft
Copilot wants to merge 2 commits into
Draft
refactor: simplify published_at parsing and fix Localization double-checked locking#174Copilot wants to merge 2 commits into
Copilot wants to merge 2 commits into
Conversation
…ation thread-safety Co-authored-by: sunzhuoshi <592211+sunzhuoshi@users.noreply.github.com> Agent-Logs-Url: https://github.com/sunzhuoshi/agent-supervisor/sessions/bb5c682f-5efd-4ed2-b038-a9783f851c02
Copilot
AI
changed the title
[WIP] Refactor code for clarity and consistency in C# source files
refactor: simplify published_at parsing and fix Localization double-checked locking
Mar 25, 2026
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.
Two targeted improvements from the code-simplifier bot suggestion, applied to the current state of
main.Changes
src/GitHubService.cs—GetLatestReleaseAsynchad 16 lines of inlinepublished_atparsing duplicating the existingTryParseJsonDateTimeprivate helper. Replaced with two lines:TryParseJsonDateTimealready defaultsresulttoDateTime.UtcNowon failure, preserving the original behaviour.src/Localization.cs— Addedvolatileto_isInitializedfor correct double-checked locking. Without it, the CPU/compiler can reorder writes so a racing thread sees_isInitialized == truebefore_resourceManagerand_currentCultureare fully visible.Out of scope
The code-simplifier branch also touched
ReviewRequestHistory.cs,ReviewRequestService.cs, andNotificationHistory.cs, but those changes were based on a commit predating theJsonFileStoreabstraction already inmain. Applying them would be a regression; those files are left unchanged.Original prompt
This section details on the original issue you should resolve
<issue_title>[code-simplifier] refactor: simplify code for clarity and consistency (2026-03-14)</issue_title>
<issue_description>This PR simplifies recently added C# source code to improve clarity, eliminate redundancy, and fix a thread-safety issue — while preserving all existing functionality.
Files Simplified
src/GitHubService.cs— reuse existing helper to eliminate duplicated parsing logicsrc/Localization.cs— fix thread-safety of double-checked lockingsrc/ReviewRequestHistory.cs— remove redundant exception message duplicationsrc/ReviewRequestService.cs— remove redundant exception message duplicationsrc/NotificationHistory.cs— remove redundant exception message duplication + unnecessary string interpolationsImprovements Made
Reduced Complexity —
src/GitHubService.csGetLatestReleaseAsynccontained 16 lines of inlinepublished_atparsing logic that duplicated what the existingTryParseJsonDateTimeprivate helper already does. Replaced with a two-line call:Thread-Safety Fix —
src/Localization.csAdded
volatileto_isInitializedfor correct double-checked locking. Withoutvolatile, the CPU/compiler can reorder writes, meaning a second thread could observe_isInitialized == truebefore the_resourceManagerand_currentCulturefields are fully visible.Consistent Logging —
ReviewRequestHistory.cs,ReviewRequestService.cs,NotificationHistory.csLogger.LogError(message, ex)already appends": {ex.Message}\n{ex.StackTrace}"to the message. Three call sites were passingex.Messagein the message string as well, causing it to be logged twice. Normalised to match the convention used throughout the rest of the codebase. Also removed needless$"..."string interpolations where no format arguments were present.Changes Based On
ebf44d— Bump gh-aw/AWF versions; target .cs (introduced all C# source files)Testing
Review Focus
Please verify:
TryParseJsonDateTimedefaultsresulttoDateTime.UtcNowon failure (confirmed in source — matches previous else-branch behaviour)volatileaddition is the correct fix for the double-checked locking patternex.MessageprefixReferences: §23091313273
📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.