Code review feedback and cleanup

dotnet · Apr 4, 2024 · 7ecfc8a · 7ecfc8a
1 parent d884497
commit 7ecfc8a
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 27 deletions.
diff --git a/src/Cli/dotnet/commands/dotnet-workload/config/WorkloadConfigCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/config/WorkloadConfigCommand.cs
@@ -20,9 +20,9 @@ namespace Microsoft.DotNet.Workloads.Workload.Config
 {
     internal class WorkloadConfigCommand : WorkloadCommandBase
     {
-        bool _hasUpdateMode;
-        string? _updateMode;
-        readonly IWorkloadResolverFactory _workloadResolverFactory;
+        private bool _hasUpdateMode;
+        private string? _updateMode;
+        private readonly IWorkloadResolverFactory _workloadResolverFactory;
 
         private string? _dotnetPath;
         private string _userProfileDir;
@@ -38,7 +38,6 @@ public WorkloadConfigCommand(
             IWorkloadResolverFactory? workloadResolverFactory = null
         ) : base(parseResult, CommonOptions.HiddenVerbosityOption, reporter)
         {
-            //  TODO: Is it possible to check the order of the options?  This would allow us to print the values out in the same order they are specified on the command line
             _hasUpdateMode = parseResult.HasOption(WorkloadConfigCommandParser.UpdateMode);
             _updateMode = parseResult.GetValue(WorkloadConfigCommandParser.UpdateMode);
 
@@ -57,13 +56,15 @@ public WorkloadConfigCommand(
 
         public override int Execute()
         {
+            //  When we support multiple configuration values, it would be nice if we could process and display them in the order they are passed.
+            //  It seems that the parser doesn't give us a good way to do that, however
             if (_hasUpdateMode)
             {
-                if (_updateMode == WorkloadConfigCommandParser.UpdateMode_WorkloadSet)
+                if (WorkloadConfigCommandParser.UpdateMode_WorkloadSet.Equals(_updateMode, StringComparison.InvariantCultureIgnoreCase))
                 {
                     _workloadInstaller.UpdateInstallMode(_sdkFeatureBand, true);
                 }
-                else if (_updateMode == WorkloadConfigCommandParser.UpdateMode_Manifests)
+                else if (WorkloadConfigCommandParser.UpdateMode_Manifests.Equals(_updateMode, StringComparison.InvariantCultureIgnoreCase))
                 {
                     _workloadInstaller.UpdateInstallMode(_sdkFeatureBand, false);
                 }

diff --git a/src/Cli/dotnet/commands/dotnet-workload/config/WorkloadConfigCommandParser.cs b/src/Cli/dotnet/commands/dotnet-workload/config/WorkloadConfigCommandParser.cs
@@ -16,7 +16,6 @@ internal static class WorkloadConfigCommandParser
         public static readonly CliOption<string> UpdateMode = new("--update-mode")
         {
             Description = LocalizableStrings.UpdateModeDescription,
-            //Hidden = true,
             Arity = ArgumentArity.ZeroOrOne
         };
 

diff --git a/src/Tests/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs b/src/Tests/dotnet-MsiInstallation.Tests/WorkloadSetTests.cs
@@ -17,26 +17,6 @@ public WorkloadSetTests(ITestOutputHelper log) : base(log)
         {
         }
 
-        //  dotnet nuget add source c:\SdkTesting\WorkloadSets
-        //  dotnet workload update --mode workloadset
-
-        //  Show workload mode in dotnet workload --info
-
-
-        //  dotnet workload update-mode set workload-set
-
-        //  dotnet workload config --update-mode workload-set
-
-        //  dotnet workload config update-mode
-        //  dotnet workload config update-mode workload-set
-        //  dotnet workload config update-mode manifests
-
-        //  dotnet workload config update-band [default|release|preview|daily]
-
-        //  dotnet config workload.update-mode workload-set
-
-        //  dotnet setconfig --workload-update-mode workload-set
-
         [Fact]
         public void DoesNotUseWorkloadSetsByDefault()
         {