From 189316ae60ecbaf88f079f520bf32036b03c361a Mon Sep 17 00:00:00 2001 From: Konrad Jamrozik Date: Tue, 6 Dec 2022 14:53:48 -0800 Subject: [PATCH] Assorted set of minor refactorings for logic related to CODEOWNERS processing (#4885) See https://github.com/Azure/azure-sdk-tools/pull/4885#issue-1478101966 --- .../ConsoleOutput.cs | 8 +++---- .../MainTests.cs | 17 +++++++------- .../Program.cs | 6 ++--- tools/code-owners-parser/CodeOwnersParser.sln | 8 ++++++- .../CodeOwnersParser.sln.DotSettings | 4 ++++ .../CodeOwnersParser/CodeOwnerEntry.cs | 3 +-- .../CodeOwnersParser/CodeOwnersFile.cs | 23 +++++-------------- .../CodeOwnersParser/FileHelpers.cs | 2 +- .../Services/GitHubService.cs | 8 ++++--- ...notification-configuration.sln.DotSettings | 3 +++ 10 files changed, 42 insertions(+), 40 deletions(-) create mode 100644 tools/code-owners-parser/CodeOwnersParser.sln.DotSettings create mode 100644 tools/notification-configuration/notification-configuration.sln.DotSettings diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs index 28d68a082c0..a4de98d7690 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/ConsoleOutput.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.IO; namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests @@ -8,8 +8,8 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests /// public class ConsoleOutput : IDisposable { - private StringWriter stringWriter; - private TextWriter originalOutput; + private readonly StringWriter stringWriter; + private readonly TextWriter originalOutput; /// /// The constructor is where we take in the console output and output to string writer. @@ -25,7 +25,7 @@ public ConsoleOutput() /// Writes the text representation of a string builder to the string. /// /// The string from console output. - public string GetOuput() + public string GetOutput() { return this.stringWriter.ToString(); } diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs index 10b4d8231d3..6242a046ec2 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners.Tests/MainTests.cs @@ -11,9 +11,9 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests [TestFixture] public class MainTests { - private const string codeOwnerFilePath = "CODEOWNERS"; + private const string CodeOwnersFilePath = "CODEOWNERS"; - private static readonly object[] _sourceLists = + private static readonly object[] sourceLists = { new object[] {"sdk", false, new List { "person1", "person2" } }, new object[] { "/sdk", false, new List { "person1", "person2" } }, @@ -24,14 +24,13 @@ public class MainTests new object[] { "/sd", true, new List() } }; - [TestCaseSource("_sourceLists")] - public void TestOnNormalOuput(string targetDirectory, bool includeUserAliasesOnly, List expectedReturn) + [TestCaseSource(nameof(sourceLists))] + public void TestOnNormalOutput(string targetDirectory, bool includeUserAliasesOnly, List expectedReturn) { - using (var consoleOutput = new ConsoleOutput()) { - Program.Main(codeOwnerFilePath, targetDirectory, includeUserAliasesOnly); - var output = consoleOutput.GetOuput(); + Program.Main(CodeOwnersFilePath, targetDirectory, includeUserAliasesOnly); + var output = consoleOutput.GetOutput(); TestExpectResult(expectedReturn, output); consoleOutput.Dispose(); } @@ -45,10 +44,10 @@ public void TestOnError(string codeOwnerPath) Assert.AreEqual(1, Program.Main(codeOwnerPath, "sdk")); } - private void TestExpectResult(List expectReturn, string output) + private static void TestExpectResult(List expectReturn, string output) { CodeOwnerEntry codeOwnerEntry = JsonSerializer.Deserialize(output); - List actualReturn = codeOwnerEntry.Owners; + List actualReturn = codeOwnerEntry!.Owners; Assert.AreEqual(expectReturn.Count, actualReturn.Count); for (int i = 0; i < actualReturn.Count; i++) { diff --git a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs index 6c94f8c678f..cb16e53bd3b 100644 --- a/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs +++ b/tools/code-owners-parser/Azure.Sdk.Tools.RetrieveCodeOwners/Program.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Text.Json; using Azure.Sdk.Tools.CodeOwnersParser; @@ -7,10 +7,10 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners /// /// The tool command to retrieve code owners. /// - public class Program + public static class Program { /// - /// Retrieves codeowners information for specific section of the repo + /// Retrieves CODEOWNERS information for specific section of the repo /// /// The path of CODEOWNERS file in repo /// The directory whose information is to be retrieved diff --git a/tools/code-owners-parser/CodeOwnersParser.sln b/tools/code-owners-parser/CodeOwnersParser.sln index 54d20fc7b39..260c393ac31 100644 --- a/tools/code-owners-parser/CodeOwnersParser.sln +++ b/tools/code-owners-parser/CodeOwnersParser.sln @@ -7,7 +7,13 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Sdk.Tools.CodeOwnersP EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Sdk.Tools.RetrieveCodeOwners", "Azure.Sdk.Tools.RetrieveCodeOwners\Azure.Sdk.Tools.RetrieveCodeOwners.csproj", "{FE65F92D-C71B-4E38-A4B2-3089EA7C5FEC}" EndProject -Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Azure.Sdk.Tools.RetrieveCodeOwners.Tests", "Azure.Sdk.Tools.RetrieveCodeOwners.Tests\Azure.Sdk.Tools.RetrieveCodeOwners.Tests.csproj", "{798B8CAC-68FC-49FD-A0F6-51C0DC4A4D1D}" +Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Azure.Sdk.Tools.RetrieveCodeOwners.Tests", "Azure.Sdk.Tools.RetrieveCodeOwners.Tests\Azure.Sdk.Tools.RetrieveCodeOwners.Tests.csproj", "{798B8CAC-68FC-49FD-A0F6-51C0DC4A4D1D}" +EndProject +Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Solution Items", "Solution Items", "{EF585CCA-55F8-44EB-921A-30996CBAFC49}" + ProjectSection(SolutionItems) = preProject + ci.yml = ci.yml + ..\..\eng\common\scripts\get-codeowners.ps1 = ..\..\eng\common\scripts\get-codeowners.ps1 + EndProjectSection EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution diff --git a/tools/code-owners-parser/CodeOwnersParser.sln.DotSettings b/tools/code-owners-parser/CodeOwnersParser.sln.DotSettings new file mode 100644 index 00000000000..1414c94c6aa --- /dev/null +++ b/tools/code-owners-parser/CodeOwnersParser.sln.DotSettings @@ -0,0 +1,4 @@ + + PR + True + True \ No newline at end of file diff --git a/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs b/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs index a81e07c7865..c2fa426734a 100644 --- a/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs +++ b/tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs @@ -1,7 +1,6 @@ -using System; +using System; using System.Collections.Generic; using System.Linq; -using System.Net.Http; namespace Azure.Sdk.Tools.CodeOwnersParser { diff --git a/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs b/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs index ce66a7ee108..578cd531e80 100644 --- a/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs +++ b/tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs @@ -1,4 +1,3 @@ -using OutputColorizer; using System; using System.Collections.Generic; using System.IO; @@ -9,20 +8,15 @@ public static class CodeOwnersFile { public static List ParseFile(string filePathOrUrl) { - string content; - content = FileHelpers.GetFileContents(filePathOrUrl); - + string content = FileHelpers.GetFileContents(filePathOrUrl); return ParseContent(content); } public static List ParseContent(string fileContent) { List entries = new List(); - string line; - // An entry ends when we get to a path (a real path or a commented dummy path) - using (StringReader sr = new StringReader(fileContent)) { CodeOwnerEntry entry = new CodeOwnerEntry(); @@ -30,7 +24,7 @@ public static List ParseContent(string fileContent) // we are going to read line by line until we find a line that is not a comment OR that is using the placeholder entry inside the comment. // while we are trying to find the folder entry, we parse all comment lines to extract the labels from it. // when we find the path or placeholder, we add the completed entry and create a new one. - while ((line = sr.ReadLine()) != null) + while (sr.ReadLine() is { } line) { line = NormalizeLine(line); @@ -93,14 +87,9 @@ public static CodeOwnerEntry FindOwnersForClosestMatch(List code } private static string NormalizeLine(string line) - { - if (string.IsNullOrEmpty(line)) - { - return line; - } - - // Remove tabs and trim extra whitespace - return line.Replace('\t', ' ').Trim(); - } + => !string.IsNullOrEmpty(line) + // Remove tabs and trim extra whitespace + ? line.Replace('\t', ' ').Trim() + : line; } } diff --git a/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs b/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs index 97a078e2946..2c9b0ff58e2 100644 --- a/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs +++ b/tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.IO; using System.Net.Http; diff --git a/tools/identity-resolution/Services/GitHubService.cs b/tools/identity-resolution/Services/GitHubService.cs index 29b48c4b588..bbd40d23e3f 100644 --- a/tools/identity-resolution/Services/GitHubService.cs +++ b/tools/identity-resolution/Services/GitHubService.cs @@ -1,4 +1,4 @@ -using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging; using System; using System.Collections.Concurrent; using System.Collections.Generic; @@ -14,8 +14,10 @@ namespace Azure.Sdk.Tools.NotificationConfiguration /// public class GitHubService { - private static HttpClient httpClient = new HttpClient(); - private static ConcurrentDictionary> codeownersFileCache = new ConcurrentDictionary>(); + private static readonly HttpClient httpClient = new HttpClient(); + + private static readonly ConcurrentDictionary> + codeownersFileCache = new ConcurrentDictionary>(); private readonly ILogger logger; diff --git a/tools/notification-configuration/notification-configuration.sln.DotSettings b/tools/notification-configuration/notification-configuration.sln.DotSettings new file mode 100644 index 00000000000..1e20132c27c --- /dev/null +++ b/tools/notification-configuration/notification-configuration.sln.DotSettings @@ -0,0 +1,3 @@ + + PR + True \ No newline at end of file