Skip to content

refactor: simplify published_at parsing and fix Localization double-checked locking#174

Draft
Copilot wants to merge 2 commits into
mainfrom
copilot/code-simplifier-refactor-simplify-code
Draft

refactor: simplify published_at parsing and fix Localization double-checked locking#174
Copilot wants to merge 2 commits into
mainfrom
copilot/code-simplifier-refactor-simplify-code

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 25, 2026

Two targeted improvements from the code-simplifier bot suggestion, applied to the current state of main.

Changes

  • src/GitHubService.csGetLatestReleaseAsync had 16 lines of inline published_at parsing duplicating the existing TryParseJsonDateTime private helper. Replaced with two lines:

    TryParseJsonDateTime(root, "published_at", out var publishedAt);
    releaseInfo.PublishedAt = publishedAt;

    TryParseJsonDateTime already defaults result to DateTime.UtcNow on failure, preserving the original behaviour.

  • src/Localization.cs — Added volatile to _isInitialized for correct double-checked locking. Without it, the CPU/compiler can reorder writes so a racing thread sees _isInitialized == true before _resourceManager and _currentCulture are fully visible.

Out of scope

The code-simplifier branch also touched ReviewRequestHistory.cs, ReviewRequestService.cs, and NotificationHistory.cs, but those changes were based on a commit predating the JsonFileStore abstraction already in main. 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 logic
  • src/Localization.cs — fix thread-safety of double-checked locking
  • src/ReviewRequestHistory.cs — remove redundant exception message duplication
  • src/ReviewRequestService.cs — remove redundant exception message duplication
  • src/NotificationHistory.cs — remove redundant exception message duplication + unnecessary string interpolations

Improvements Made

Reduced Complexity — src/GitHubService.cs

GetLatestReleaseAsync contained 16 lines of inline published_at parsing logic that duplicated what the existing TryParseJsonDateTime private helper already does. Replaced with a two-line call:

// Before (16 lines)
if (root.TryGetProperty("published_at", out var published))
{
    var publishedStr = published.GetString();
    if (!string.IsNullOrEmpty(publishedStr) && DateTime.TryParse(publishedStr, out var publishedAt))
        releaseInfo.PublishedAt = publishedAt;
    else
        releaseInfo.PublishedAt = DateTime.UtcNow;
}
else
    releaseInfo.PublishedAt = DateTime.UtcNow;

// After (2 lines)
TryParseJsonDateTime(root, "published_at", out var publishedAt);
releaseInfo.PublishedAt = publishedAt;

Thread-Safety Fix — src/Localization.cs

Added volatile to _isInitialized for correct double-checked locking. Without volatile, the CPU/compiler can reorder writes, meaning a second thread could observe _isInitialized == true before the _resourceManager and _currentCulture fields are fully visible.

// Before
private static bool _isInitialized = false;

// After
private static volatile bool _isInitialized = false;

Consistent Logging — ReviewRequestHistory.cs, ReviewRequestService.cs, NotificationHistory.cs

Logger.LogError(message, ex) already appends ": {ex.Message}\n{ex.StackTrace}" to the message. Three call sites were passing ex.Message in 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

  • Commit ebf44dBump gh-aw/AWF versions; target .cs (introduced all C# source files)

Testing

  • ✅ No functional changes — behaviour is identical
  • ✅ Build verified via CI (Windows runner required for WinForms target)
  • ✅ No linting issues introduced

Review Focus

Please verify:

  • TryParseJsonDateTime defaults result to DateTime.UtcNow on failure (confirmed in source — matches previous else-branch behaviour)
  • volatile addition is the correct fix for the double-checked locking pattern
  • Logging messages remain meaningful without the redundant ex.Message prefix

References: §23091313273

Generated by Code Simplifier ·

To install this agentic workflow, run

gh aw add github/gh-aw/.github/workflows/code-simplifier.md@852cb06ad52958b402ed982b69957ffc57ca0619

[!WARNING]

⚠️ Firewall blocked 2 domains

The following domains were blocked by the firewall during workflow execution:

  • api.nuget.org
  • dc.services.visualstudio.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "api.nuget.org"
    - "dc.services.visualstudio.com"

See Network Configuration for more information.

  • expires on Mar 15, 2026, 4:05 PM UTC

[!NOTE]
This was originally intended as a pull request, but GitHub Actions is not permitted to create or approve pull requests in this repository.
The changes have been...


📱 Kick off Copilot coding agent tasks wherever you are with GitHub Mobile, available on iOS and Android.

…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
Copilot AI requested a review from sunzhuoshi March 25, 2026 11:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[code-simplifier] refactor: simplify code for clarity and consistency (2026-03-14)

2 participants