Update and Refactor Codebase: Improve Async Usage, Package Management…

…, and Code Quality (#20) ## Summary This pull request includes a series of updates and refactoring efforts across the codebase, focusing on improving async usage, introducing central package management, updating package references, and enhancing code quality and readability. ## Changes - Remove unnecessary TODO comment and severity setting in .editorconfig - Set dotnet_diagnostic.CA2201.severity to none in .editorconfig - Add dotnet_diagnostic.VSTHRD200.severity setting in .editorconfig - Create Directory.Build.targets file to enable central package management and run 'dotnet format' on dev machines during Release builds - Create Directory.Packages.props file to centrally manage package versions and include analyzers - Enable central package management in Directory.Packages.props - Add package versions and package references for analyzers in Directory.Packages.props - Replace synchronous ReadToEnd with async ReadToEndAsync in multiple files - Remove unnecessary newline characters in ConfigCommand.cs - Change ConfigConstants class to static - Update package references in SKonsole.csproj to use the latest versions without specifying version numbers - Make GitDiffStaged method in GitSkill.cs async - Update CondenseSkill.cs to use 'this' keyword for better readability - Add RootNamespace to CondenseSkill.csproj - Updated CondenseSkill.csproj and PRSkill.csproj files - Refactored EnglishRobertaTokenizer.cs by making the class sealed - Removed unnecessary using directives in PullRequestSkill.cs - Updated PackageReference for Microsoft.SemanticKernel in PRSkill.csproj - Simplified object creation in RedirectTextCompletionResult.cs - Removed unnecessary whitespace and newline characters in various files - Made FormatInstructionsProvider a static class - Updated the output format instructions in FormatInstructionsProvider.cs --- *Powered by [Microsoft Semantic Kernel](https://github.com/microsoft/semantic-kernel)* * 🔧 Adjust .editorconfig settings and disable some diagnostics Update the .editorconfig file to disable some diagnostics, such as CA2201 and VSTHRD200, and remove the unnecessary severity setting for dotnet_analyzer_diagnostic. This change aims to improve code quality and maintainability while reducing unwanted changes introduced by "dotnet format". * 📦 Centralize package management and format on build Implement Central Package Management using Directory.Packages.props and Directory.Build.targets. This centralizes package versions and analyzers across all projects. Also, add a target to run 'dotnet format' on build for Release configurations in dev environments. Update the Microsoft.SemanticKernel package reference in PRSkill.csproj to use the latest version. This change ensures compatibility and performance improvements for the project. * 🔄 Replace ReadToEnd with ReadToEndAsync in commands This commit replaces the synchronous ReadToEnd method with the asynchronous ReadToEndAsync method in CommitCommand and PRCommand classes. It also makes ConfigConstants class static and removes unnecessary newlines in ConfigCommand. * 🔄 Update GitDiffStaged to use async-await This commit updates the GitDiffStaged method in GitSkill.cs to use async-await for better performance and readability. The method now returns an awaitable Task and reads the process output asynchronously. * 📝 Improve CondenseSkill and tokenizer consistency - Update method call to use `this` keyword for consistency - Add BOM to resource files for proper encoding - Change EnglishRobertaTokenizer class to sealed * 🔧 Refactor PRSkill and Utils classes - Remove unnecessary usings and reorder them - Simplify object initialization - Make FormatInstructionsProvider static - Minor formatting adjustments
lemillermicrosoft · Sep 19, 2023 · 59dc149 · 59dc149
1 parent eab8cba
commit 59dc149
Show file tree

Hide file tree

Showing 22 changed files with 124 additions and 60 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -95,9 +95,6 @@ dotnet_code_quality_unused_parameters = all:suggestion
 
 [*.cs]
 
-# TODO: enable this but stop "dotnet format" from applying incorrect fixes and introducing a lot of unwanted changes.
-dotnet_analyzer_diagnostic.severity = none
-
 # Note: these settings cause "dotnet format" to fix the code. You should review each change if you uses "dotnet format".
 dotnet_diagnostic.RCS1036.severity = warning # Remove unnecessary blank line.
 dotnet_diagnostic.RCS1037.severity = warning # Remove trailing white-space.
@@ -119,6 +116,8 @@ dotnet_diagnostic.RCS1232.severity = warning # Order elements in documentation c
 # Commented out because `dotnet format` removes the xmldoc element, while we should add the missing documentation instead.
 # dotnet_diagnostic.RCS1228.severity = warning # Unused element in documentation comment.
 
+dotnet_diagnostic.CA2201.severity = none # Exception type System.Exception is not sufficiently specific
+
 # Diagnostics elevated as warnings
 dotnet_diagnostic.CA1000.severity = warning # Do not declare static members on generic types
 dotnet_diagnostic.CA1031.severity = warning # Do not catch general exception types
@@ -131,7 +130,6 @@ dotnet_diagnostic.CA1852.severity = warning # Sealed classes
 dotnet_diagnostic.CA1859.severity = warning # Use concrete types when possible for improved performance
 dotnet_diagnostic.CA1860.severity = warning # Prefer comparing 'Count' to 0 rather than using 'Any()', both for clarity and for performance
 dotnet_diagnostic.CA2000.severity = warning # Call System.IDisposable.Dispose on object before all references to it are out of scope
-dotnet_diagnostic.CA2201.severity = warning # Exception type System.Exception is not sufficiently specific
 
 dotnet_diagnostic.IDE0001.severity = warning # Simplify name
 dotnet_diagnostic.IDE0005.severity = warning # Remove unnecessary using directives
@@ -171,6 +169,7 @@ dotnet_diagnostic.CA2227.severity = none # Change to be read-only by removing th
 dotnet_diagnostic.CA2253.severity = none # Named placeholders in the logging message template should not be comprised of only numeric characters
 
 dotnet_diagnostic.VSTHRD111.severity = none # Use .ConfigureAwait(bool) is hidden by default, set to none to prevent IDE from changing on autosave
+dotnet_diagnostic.VSTHRD200.severity = none # Use Async suffix for async methods
 dotnet_diagnostic.xUnit1004.severity = none # Test methods should not be skipped. Remove the Skip property to start running the test again.
 
 dotnet_diagnostic.RCS1021.severity = none # Use expression-bodied lambda.
@@ -243,7 +242,6 @@ dotnet_naming_style.uppercase_with_underscore_separator.capitalization = all_upp
 dotnet_naming_style.uppercase_with_underscore_separator.word_separator = _
 
 dotnet_naming_style.end_in_async.required_prefix =
-dotnet_naming_style.end_in_async.required_suffix = Async
 dotnet_naming_style.end_in_async.capitalization = pascal_case
 dotnet_naming_style.end_in_async.word_separator =
 

diff --git a/Directory.Build.targets b/Directory.Build.targets
@@ -0,0 +1,13 @@
+<Project>
+  <!-- Direct all packages under 'dotnet' to get versions from Directory.Packages.props -->
+  <!-- using Central Package Management feature -->
+  <!-- https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management -->
+  <Sdk Name="Microsoft.Build.CentralPackageVersions" Version="2.1.3" />
+  <!-- Only run 'dotnet format' on dev machines, Release builds. Skip on GitHub Actions -->
+  <!-- as this runs in its own Actions job. -->
+  <Target Name="DotnetFormatOnBuild" BeforeTargets="Build"
+          Condition=" '$(Configuration)' == 'Release' AND '$(GITHUB_ACTIONS)' == '' ">
+    <Message Text="Running dotnet format" Importance="high" />
+    <Exec Command="dotnet format --no-restore -v diag $(ProjectFileName)" />
+  </Target>
+</Project>
diff --git a/Directory.Packages.props b/Directory.Packages.props
@@ -0,0 +1,60 @@
+<Project>
+  <PropertyGroup>
+    <!-- Enable central package management -->
+    <!-- https://learn.microsoft.com/en-us/nuget/consume-packages/Central-Package-Management -->
+    <ManagePackageVersionsCentrally>true</ManagePackageVersionsCentrally>
+  </PropertyGroup>
+  <ItemGroup>
+    <PackageVersion Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0" />
+    <PackageVersion Include="Microsoft.Extensions.Logging.Console" Version="7.0.0" />
+    <PackageVersion Include="Microsoft.ML.Tokenizers" Version="0.21.0-preview.23266.6"/>
+    <PackageVersion Include="Microsoft.SemanticKernel" Version="0.24.230918.1-preview" />
+    <PackageVersion Include="Microsoft.SemanticKernel.Skills.Web" Version="0.24.230918.1-preview" />
+    <PackageVersion Include="Microsoft.SemanticKernel.Planning.StepwisePlanner" Version="0.24.230918.1-preview" />
+    <PackageVersion Include="Microsoft.SemanticKernel.Reliability.Basic" Version="0.24.230918.1-preview" />
+    <PackageVersion Include="Spectre.Console" Version="0.47.0" />
+    <PackageVersion Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
+    <PackageVersion Include="TextCopy" Version="6.2.1" />
+    <!-- Analyzers -->
+    <PackageVersion Include="Microsoft.CodeAnalysis.NetAnalyzers" Version="8.0.0-preview1.23165.1" />
+    <PackageReference Include="Microsoft.CodeAnalysis.NetAnalyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Microsoft.CodeAnalysis.Analyzers" Version="3.3.4" />
+    <PackageReference Include="Microsoft.CodeAnalysis.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Microsoft.VisualStudio.Threading.Analyzers" Version="17.7.30" />
+    <PackageReference Include="Microsoft.VisualStudio.Threading.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="xunit.analyzers" Version="1.2.0" />
+    <PackageReference Include="xunit.analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Moq.Analyzers" Version="0.0.9" />
+    <PackageReference Include="Moq.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Roslynator.Analyzers" Version="4.3.0" />
+    <PackageReference Include="Roslynator.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Roslynator.CodeAnalysis.Analyzers" Version="4.3.0" />
+    <PackageReference Include="Roslynator.CodeAnalysis.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+    <PackageVersion Include="Roslynator.Formatting.Analyzers" Version="4.3.0" />
+    <PackageReference Include="Roslynator.Formatting.Analyzers">
+      <PrivateAssets>all</PrivateAssets>
+      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
+    </PackageReference>
+  </ItemGroup>
+</Project>
diff --git a/apps/SKonsole/Commands/CommitCommand.cs b/apps/SKonsole/Commands/CommitCommand.cs
@@ -61,7 +61,7 @@ private static async Task RunCommitMessage(CancellationToken token, ILogger logg
                 }
             };
             process.Start();
-            output = process.StandardOutput.ReadToEnd();
+            output = await process.StandardOutput.ReadToEndAsync();
         }
         else
         {
@@ -78,7 +78,7 @@ private static async Task RunCommitMessage(CancellationToken token, ILogger logg
                 }
             };
             process.Start();
-            output = process.StandardOutput.ReadToEnd();
+            output = await process.StandardOutput.ReadToEndAsync();
             if (string.IsNullOrEmpty(output))
             {
                 using var retryProcess = new Process
@@ -94,7 +94,7 @@ private static async Task RunCommitMessage(CancellationToken token, ILogger logg
                     }
                 };
                 retryProcess.Start();
-                output = retryProcess.StandardOutput.ReadToEnd();
+                output = await retryProcess.StandardOutput.ReadToEndAsync();
             }
         }
 
@@ -126,7 +126,7 @@ void HorizontalRule(string title, string style = "white bold")
                 return result;
             });
 
-        AnsiConsole.WriteLine(botMessage.ToString());
+        AnsiConsole.WriteLine(botMessage);
         HorizontalRule(string.Empty);
     }
 

diff --git a/apps/SKonsole/Commands/ConfigCommand.cs b/apps/SKonsole/Commands/ConfigCommand.cs
@@ -115,13 +115,11 @@ await ConfigOrExitAsync(keys,
             {
                 await config.SaveConfig(configKey, value.Trim());
             }
-
         }, cancellationToken);
     }
 
     private static async Task LLMConfigAsync(CancellationToken cancellationToken)
     {
-
         await ConfigOrExitAsync(new[]
             {
                 ConfigConstants.AzureOpenAI,

diff --git a/apps/SKonsole/Commands/PRCommand.cs b/apps/SKonsole/Commands/PRCommand.cs
@@ -104,7 +104,7 @@ private static async Task RunPullRequestFeedback(CancellationToken token, ILogge
         };
         process.Start();
 
-        string output = process.StandardOutput.ReadToEnd();
+        string output = await process.StandardOutput.ReadToEndAsync();
 
         var pullRequestSkill = kernel.ImportSkill(new PRSkill.PullRequestSkill(kernel));
 
@@ -156,7 +156,7 @@ private static async Task<string> FetchDiff(string targetBranch, string diffInpu
             };
             process.Start();
 
-            return process.StandardOutput.ReadToEnd();
+            return await process.StandardOutput.ReadToEndAsync();
         }
         else if (diffInputFile.StartsWith("http"))
         {

diff --git a/apps/SKonsole/ConfigConstants.cs b/apps/SKonsole/ConfigConstants.cs
@@ -1,6 +1,6 @@
 namespace SKonsole;
 
-public class ConfigConstants
+public static class ConfigConstants
 {
     public const string OPENAI_CHAT_MODEL_ID = "OPENAI_CHAT_MODEL_ID";
 

diff --git a/apps/SKonsole/SKonsole.csproj b/apps/SKonsole/SKonsole.csproj
@@ -14,15 +14,15 @@
     <ToolCommandName>skonsole</ToolCommandName>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" Version="7.0.0" />
-    <PackageReference Include="Microsoft.Extensions.Logging.Console" Version="7.0.0" />
-    <PackageReference Include="Microsoft.SemanticKernel" Version="0.24.230918.1-preview" />
-    <PackageReference Include="Microsoft.SemanticKernel.Skills.Web" Version="0.24.230918.1-preview" />
-    <PackageReference Include="Microsoft.SemanticKernel.Planning.StepwisePlanner" Version="0.24.230918.1-preview" />
-    <PackageReference Include="Microsoft.SemanticKernel.Reliability.Basic" Version="0.24.230918.1-preview" />
-    <PackageReference Include="Spectre.Console" Version="0.47.0" />
-    <PackageReference Include="System.CommandLine" Version="2.0.0-beta4.22272.1" />
-    <PackageReference Include="TextCopy" Version="6.2.1" />
+    <PackageReference Include="Microsoft.Extensions.Configuration.EnvironmentVariables" />
+    <PackageReference Include="Microsoft.Extensions.Logging.Console" />
+    <PackageReference Include="Microsoft.SemanticKernel"/>
+    <PackageReference Include="Microsoft.SemanticKernel.Skills.Web"/>
+    <PackageReference Include="Microsoft.SemanticKernel.Planning.StepwisePlanner"/>
+    <PackageReference Include="Microsoft.SemanticKernel.Reliability.Basic"/>
+    <PackageReference Include="Spectre.Console"/>
+    <PackageReference Include="System.CommandLine"  />
+    <PackageReference Include="TextCopy" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\..\skills\PRSkill\PRSkill.csproj" />

diff --git a/apps/SKonsole/Skills/GitSkill.cs b/apps/SKonsole/Skills/GitSkill.cs
@@ -8,7 +8,7 @@ namespace SKonsole.Skills;
 internal sealed class GitSkill
 {
     [SKFunction, Description("Run 'git diff --staged' and return it's output.")]
-    public static Task<SKContext> GitDiffStaged(SKContext context)
+    public static async Task<SKContext> GitDiffStaged(SKContext context)
     {
         using var process = new Process
         {
@@ -24,8 +24,8 @@ public static Task<SKContext> GitDiffStaged(SKContext context)
         };
         process.Start();
 
-        string output = process.StandardOutput.ReadToEnd();
+        string output = await process.StandardOutput.ReadToEndAsync();
         context.Variables.Update(output);
-        return Task.FromResult(context);
+        return context;
     }
 }
diff --git a/skills/CondenseSkill/CondenseSkill.cs b/skills/CondenseSkill/CondenseSkill.cs
@@ -60,7 +60,7 @@ public async Task<SKContext> Condense(
 
         // update memory with serialized list of results and call condense again
         this._logger.LogWarning($"Condensing {paragraphs.Count} paragraphs");
-        return await Condense(context, string.Join("\n", condenseResult), RESULTS_SEPARATOR);
+        return await this.Condense(context, string.Join("\n", condenseResult), RESULTS_SEPARATOR);
     }
 
     private static string CondenseSkillPath()

diff --git a/skills/CondenseSkill/CondenseSkill.csproj b/skills/CondenseSkill/CondenseSkill.csproj
@@ -1,12 +1,13 @@
-<Project Sdk="Microsoft.NET.Sdk">
+<Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFramework>netstandard2.1</TargetFramework>
     <ImplicitUsings>enable</ImplicitUsings>
     <LangVersion>10</LangVersion>
+    <RootNamespace>CondenseSkillLib</RootNamespace>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="Microsoft.SemanticKernel" Version="0.24.230918.1-preview" />
-    <PackageReference Include="Microsoft.ML.Tokenizers" Version="0.21.0-preview.23266.6"/>
+    <PackageReference Include="Microsoft.SemanticKernel"/>
+    <PackageReference Include="Microsoft.ML.Tokenizers"/>
   </ItemGroup>
   <ItemGroup>
     <None Update="SemanticFunctions/**/*">

diff --git a/skills/CondenseSkill/Resources/dict.txt b/skills/CondenseSkill/Resources/dict.txt
@@ -1,4 +1,4 @@
-13 850314647
+13 850314647
 262 800385005
 11 800251374
 284 432911125

diff --git a/skills/CondenseSkill/Resources/encoder.json b/skills/CondenseSkill/Resources/encoder.json
@@ -1,4 +1,4 @@
-{
+{
   "!": 0,
   "\"": 1,
   "#": 2,

diff --git a/skills/CondenseSkill/Resources/vocab.bpe b/skills/CondenseSkill/Resources/vocab.bpe
@@ -1,4 +1,4 @@
-#version: 0.2
+#version: 0.2
 Ġ t
 Ġ a
 h e

diff --git a/skills/CondenseSkill/SemanticFunctions/CondenseSkill/Condenser/config.json b/skills/CondenseSkill/SemanticFunctions/CondenseSkill/Condenser/config.json
@@ -1,4 +1,4 @@
-{
+{
   "schema": 1,
   "type": "completion",
   "description": "Condense multiple pieces of various texts into a single output.",

diff --git a/skills/CondenseSkill/SemanticFunctions/CondenseSkill/Condenser/skprompt.txt b/skills/CondenseSkill/SemanticFunctions/CondenseSkill/Condenser/skprompt.txt
@@ -1,4 +1,4 @@
-[RESULTS]
+[RESULTS]
 {{$input}}
 [END RESULTS]
 

diff --git a/skills/CondenseSkill/Tokenizers/EnglishRobertaTokenizer.cs b/skills/CondenseSkill/Tokenizers/EnglishRobertaTokenizer.cs
@@ -1,10 +1,10 @@
-using System.Reflection;
+using System.Reflection;
 using Microsoft.ML.Tokenizers;
 using static Microsoft.SemanticKernel.Text.TextChunker;
 
 namespace CondenseSkillLib.Tokenizers;
 
-internal class EnglishRobertaTokenizer
+internal sealed class EnglishRobertaTokenizer
 {
     internal static TokenCounter Counter => (string input) =>
     {

diff --git a/skills/PRSkill/PRSkill.csproj b/skills/PRSkill/PRSkill.csproj
@@ -5,7 +5,7 @@
     <LangVersion>10</LangVersion>
   </PropertyGroup>
   <ItemGroup>
-    <PackageReference Include="Microsoft.SemanticKernel" Version="0.24.230918.1-preview" />
+    <PackageReference Include="Microsoft.SemanticKernel" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="..\CondenseSkill\CondenseSkill.csproj" />

diff --git a/skills/PRSkill/PullRequestSkill.cs b/skills/PRSkill/PullRequestSkill.cs
@@ -1,16 +1,14 @@
-// Copyright (c) Microsoft. All rights reserved.
+// Copyright (c) Microsoft. All rights reserved.
 
+using System.ComponentModel;
 using System.Reflection;
+using CondenseSkillLib;
 using Microsoft.Extensions.Logging;
-using PRSkill.Utils;
 using Microsoft.SemanticKernel;
+using Microsoft.SemanticKernel.AI.TextCompletion;
 using Microsoft.SemanticKernel.Orchestration;
 using Microsoft.SemanticKernel.SkillDefinition;
-using CondenseSkillLib;
-using Microsoft.SemanticKernel.AI.TextCompletion;
-using System.ComponentModel;
-using System.Net.WebSockets;
-using System.Text.Json.Nodes;
+using PRSkill.Utils;
 
 namespace PRSkill;
 
@@ -208,7 +206,7 @@ public RedirectTextCompletionResult(string completion)
         this._completion = completion;
     }
 
-    public ModelResult ModelResult => new ModelResult(this._completion);
+    public ModelResult ModelResult => new(this._completion);
 
     public Task<string> GetCompletionAsync(CancellationToken cancellationToken = default)
     {

diff --git a/skills/PRSkill/Utils/CommitChunker.cs b/skills/PRSkill/Utils/CommitChunker.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft. All rights reserved.
+// Copyright (c) Microsoft. All rights reserved.
 
 namespace PRSkill.Utils;
 

diff --git a/skills/PRSkill/Utils/CommitParser.cs b/skills/PRSkill/Utils/CommitParser.cs
@@ -1,4 +1,4 @@
-// Copyright (c) Microsoft. All rights reserved.
+// Copyright (c) Microsoft. All rights reserved.
 
 using System.Text.RegularExpressions;
 
@@ -55,7 +55,6 @@ internal static class StringEx
         return fileDiffChunks;
     }
 
-
     internal static List<string> SplitByRegex(this string input, string pattern, RegexOptions options)
     {
         var matches = Regex.Matches(input, pattern, options);
@@ -84,5 +83,4 @@ internal static List<string> SplitByRegex(this string input, string pattern, Reg
 
         return chunks;
     }
-
 }