Skip to content

Address PR review: cleanup, error propagation, and remove build artifacts#4

Draft
Copilot wants to merge 2 commits into
copilot/convert-to-wpf-using-net8from
copilot/sub-pr-3
Draft

Address PR review: cleanup, error propagation, and remove build artifacts#4
Copilot wants to merge 2 commits into
copilot/convert-to-wpf-using-net8from
copilot/sub-pr-3

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Apr 7, 2026

Addresses all review comments from the WPF .NET 8 rewrite PR. Key issues: build artifacts tracked in git, App.config coexisting with appsettings.json, silent copy failures allowing install to proceed on incomplete files, and minor code quality issues.

Changes

  • Error propagationOverWriteDestinationFile now rethrows after logging so Complete/Install modes abort if file copy fails instead of silently continuing to the install step
  • Bat file extraction — replaced Regex.Match + string.Replace with Path.GetFileName(); removed System.Text.RegularExpressions import
  • Null filteringGetFolderName changed from .Where(n => n != null)! to .Where(n => !string.IsNullOrEmpty(n)).Select(n => n!), eliminating the null-forgiving suppression
  • JsonDocument disposalLoadSettings now uses using var doc = JsonDocument.Parse(json) to release pooled buffers promptly
  • App.config removed — deleted legacy .NET Framework config; appsettings.json is the sole config source
  • .gitignore + untrack build artifacts — added .gitignore covering bin/, obj/, .vs/, *.user; removed all previously committed build outputs from tracking

…r propagation, IDisposable, remove App.config and build artifacts

Agent-Logs-Url: https://github.com/gb63728f4/UpdateService/sessions/efd0bd1d-6f71-46ec-92a6-9920b4170156

Co-authored-by: gb63728f4 <39524431+gb63728f4@users.noreply.github.com>
Copilot AI changed the title [WIP] Rewrite UpdateService as WPF .NET 8 Address PR review: cleanup, error propagation, and remove build artifacts Apr 7, 2026
Copilot AI requested a review from gb63728f4 April 7, 2026 08:40
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.

2 participants