cleanup: remove business fields from UpdateOptions, _customOptions, ClientCoreTest, Linux Script#393
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR performs a multi-area cleanup across GeneralUpdate.Core by removing business-specific configuration from UpdateOptions, removing the legacy “custom options” pre-check mechanism, deleting the ClientCoreTest test project, and removing LinuxStrategy’s built-in script execution in favor of the existing hooks system.
Changes:
- Removed business fields from
UpdateOptionsto keep it framework-level only, and updated bootstrap usage accordingly. - Removed
_customOptions/AddCustomOption/UseCustomOptionsupport from bootstrap and strategy workflow. - Removed LinuxStrategy startup script execution and deleted the
tests/ClientCoreTestproject; added/expanded XML docs on several configuration types.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/ClientCoreTest/README.md | Removes documentation for the deleted ClientCoreTest project. |
| tests/ClientCoreTest/Hubs/UpgradeHubServiceTests.cs | Deletes hub service tests along with the removed test project. |
| tests/ClientCoreTest/Hubs/RandomRetryPolicyTests.cs | Deletes retry policy tests along with the removed test project. |
| tests/ClientCoreTest/ClientCoreTest.csproj | Deletes the ClientCoreTest project definition. |
| tests/ClientCoreTest/Bootstrap/ClientBootstrapScenarioTests.cs | Deletes scenario tests that depended on custom options and script behavior. |
| src/c#/GeneralUpdate.Core/Strategy/WindowsStrategy.cs | Removes duplicate/unused using directives. |
| src/c#/GeneralUpdate.Core/Strategy/LinuxStrategy.cs | Removes ExecuteScript() and adjusts startup logging to launch the main app directly. |
| src/c#/GeneralUpdate.Core/Strategy/ClientUpdateStrategy.cs | Removes custom option registration/execution from the client workflow. |
| src/c#/GeneralUpdate.Core/Strategy/AbstractStrategy.cs | Removes duplicate using directive. |
| src/c#/GeneralUpdate.Core/Configuration/UpdateOptions.cs | Removes business options; documents and retains framework-level options. |
| src/c#/GeneralUpdate.Core/Configuration/UpdateMode.cs | Adds XML docs to the enum. |
| src/c#/GeneralUpdate.Core/Configuration/ReportType.cs | Adds XML docs to report status constants. |
| src/c#/GeneralUpdate.Core/Configuration/Format.cs | Adds XML docs to compression format constants. |
| src/c#/GeneralUpdate.Core/Configuration/DiffMode.cs | Expands enum and adds XML docs. |
| src/c#/GeneralUpdate.Core/Bootstrap/GeneralUpdateBootstrap.cs | Removes custom options wiring and updates hub auth values to come from config info. |
Comments suppressed due to low confidence (1)
tests/ClientCoreTest/ClientCoreTest.csproj:1
- This project is removed, but the repo still references it in
src/c#/GeneralUpdate.slnxand in.github/workflows/ci.yml(Ubuntu test step runsdotnet test tests/ClientCoreTest/ClientCoreTest.csproj). Those references need to be removed/updated or CI and solution builds will fail due to the missing csproj.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+29
to
30
| /// <summary>Download timeout in seconds.</summary> | ||
| public static UpdateOption<int?> DownloadTimeout { get; } = UpdateOption.ValueOf<int?>("DOWNLOADTIMEOUT", 30); |
Comment on lines
+5
to
+10
| /// </summary> | ||
| public enum UpdateMode | ||
| { | ||
| /// <summary>Standard file-based update.</summary> | ||
| Default = 0, | ||
| /// <summary>Script-based custom update logic.</summary> |
…lientCoreTest, Linux Script 1. UpdateOptions.cs: remove 14 business fields (UpdateUrl, Token, Scheme, etc) - keep only 20 framework-level options (AppType, Silent, DiffMode, etc.) - business fields already exist in Configinfo/BaseConfigInfo 2. Remove _customOptions mechanism from GeneralUpdateBootstrap + ClientUpdateStrategy - hooks (IUpdateHooks) already cover pre/post-update logic - Hub config now uses _configInfo.Token/AppSecretKey 3. Delete tests/ClientCoreTest/ (no longer needed) 4. LinuxStrategy: remove Script mechanism - IUpdateHooks.OnBeforeStartAppAsync covers permission-setting 5. GeneralUpdate.Core cleanup: unused usings, XML docs on enums
- BootstrapFullParameterMatrixTests: remove all tests/options for deleted business fields (Deployment/Security/Reporting regions), remove AddCustomOption calls, keep only framework-level options - BootstrapHooksAndExtensionsTests: remove AddCustomOption and deleted UpdateOptions references, consolidate test code
JusterZhu
force-pushed
the
cleanup/remove-customoptions-bizfields
branch
from
5e80d67 to
f10b247
Compare
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.
Summary
5-in-1 cleanup per juster's requirements:
1. UpdateOptions.cs — Business fields removed
Removed 14 business fields that already exist in \Configinfo/\BaseConfigInfo:
\UpdateUrl, \AppSecretKey, \AppName, \MainAppName, \InstallPath, \ClientVersion, \UpgradeClientVersion, \Platform, \ReportUrl, \ProductId, \PermissionScript, \Scheme, \Token, \Bowl, \UpdateLogUrl, \Script\
Kept 20 framework-level options: \AppType, \DiffMode, \Encoding, \Format, \DownloadTimeout, \DriveEnabled, \PatchEnabled, \BackupEnabled, \Mode, \Silent, \SilentAutoInstall, \SilentPollIntervalMinutes, \MaxConcurrency, \EnableResume, \RetryCount, \VerifyChecksum, \RetryInterval, \OSSProvider, \OSSBucketRegion, \BlackList, \Hub\
2. _customOptions mechanism removed
3. ClientCoreTest project deleted
4. LinuxStrategy Script mechanism removed
5. GeneralUpdate.Core cleanup
Results