Conversation
There was a problem hiding this comment.
Pull request overview
This pull request aims to adapt the codebase to .NET 10 by upgrading NuGet package versions from 9.0.0 to 10.0.1 and introducing new features including a Scripts-based update mode. The changes refactor the GeneralUpdateBootstrap class to support two execution modes (Default and Scripts) and convert WindowsStrategy from synchronous to asynchronous execution.
Key changes include:
- Upgrade of multiple NuGet packages (System.Text.Json, Microsoft.Bcl.AsyncInterfaces, System.Collections.Immutable, Microsoft.AspNetCore.SignalR.Client) from version 9.0.0 to 10.0.1
- Introduction of UpdateMode enum with Default and Scripts modes, enabling workflow-based update execution
- Significant refactoring of GeneralUpdateBootstrap to support both environment-based initialization and explicit configuration via SetConfig method
- Conversion of WindowsStrategy.Execute to ExecuteAsync pattern, removing Task.Run wrapper
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 18 comments.
Show a summary per file
| File | Description |
|---|---|
| src/c#/GeneralUpdate.Upgrad/Program.cs | Adds explicit configuration setup using Configinfo and new UpdateMode.Scripts option |
| src/c#/GeneralUpdate.Upgrad/GeneralUpdate.Upgrad.csproj | Updates System.Text.Json package from 9.0.0 to 10.0.1 |
| src/c#/GeneralUpdate.Core/Strategys/WindowsStrategy.cs | Converts Execute method to ExecuteAsync, removing Task.Run wrapper for cleaner async pattern |
| src/c#/GeneralUpdate.Core/GeneralUpdateBootstrap.cs | Major refactoring: adds SetConfig method, ExecuteWorkflowAsync for Scripts mode, helper methods, and reorganizes code structure |
| src/c#/GeneralUpdate.Core/GeneralUpdate.Core.csproj | Updates three packages to version 10.0.1 and adds UpdateMode.cs link |
| src/c#/GeneralUpdate.Common/Shared/Object/Enum/UpdateMode.cs | Adds new UpdateMode enum with Default and Scripts values |
| src/c#/GeneralUpdate.Common/Internal/Bootstrap/UpdateOption.cs | Adds Mode option for UpdateMode configuration |
| src/c#/GeneralUpdate.Common/GeneralUpdate.Common.csproj | Updates three packages to version 10.0.1 |
| src/c#/GeneralUpdate.ClientCore/GeneralUpdate.ClientCore.csproj | Updates three packages to version 10.0.1 and adds UpdateMode.cs link |
| src/c#/GeneralUpdate.Bowl/GeneralUpdate.Bowl.csproj | Updates two packages to version 10.0.1 and adds UpdateMode.cs link |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public override void Create(GlobalConfigInfo parameter) => _configinfo = parameter; | ||
|
|
||
| public override void Execute() | ||
| public override async Task ExecuteAsync() |
There was a problem hiding this comment.
The WindowsStrategy has been updated to use ExecuteAsync, but LinuxStrategy in the same directory still uses the synchronous Execute method with Task.Run. For consistency and maintainability, both strategies should follow the same async pattern. Consider updating LinuxStrategy to also implement ExecuteAsync instead of Execute.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@JusterZhu I've opened a new pull request, #96, to work on those changes. Once the pull request is ready, I'll request review from you. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// <summary> | ||
| /// Specifies the update execution mode. | ||
| /// </summary> | ||
| public static readonly UpdateOption<UpdateMode?> Mode = ValueOf<UpdateMode?>("MODE"); |
There was a problem hiding this comment.
The documentation comment for the Mode field is copy-pasted from the BackUp field above it and incorrectly states "Whether to enable the backup function" when it should describe the update mode setting.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@JusterZhu I've opened a new pull request, #97, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 10 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
No description provided.