Skip to content

Commit

Permalink
Assorted set of minor refactorings for logic related to CODEOWNERS pr…
Browse files Browse the repository at this point in the history
…ocessing (#4885)

See #4885 (comment)
  • Loading branch information
Konrad Jamrozik authored Dec 6, 2022
1 parent 8581859 commit 189316a
Show file tree
Hide file tree
Showing 10 changed files with 42 additions and 40 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.IO;

namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests
Expand All @@ -8,8 +8,8 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners.Tests
/// </summary>
public class ConsoleOutput : IDisposable
{
private StringWriter stringWriter;
private TextWriter originalOutput;
private readonly StringWriter stringWriter;
private readonly TextWriter originalOutput;

/// <summary>
/// The constructor is where we take in the console output and output to string writer.
Expand All @@ -25,7 +25,7 @@ public ConsoleOutput()
/// Writes the text representation of a string builder to the string.
/// </summary>
/// <returns>The string from console output.</returns>
public string GetOuput()
public string GetOutput()
{
return this.stringWriter.ToString();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> { "person1", "person2" } },
new object[] { "/sdk", false, new List<string> { "person1", "person2" } },
Expand All @@ -24,14 +24,13 @@ public class MainTests
new object[] { "/sd", true, new List<string>() }
};

[TestCaseSource("_sourceLists")]
public void TestOnNormalOuput(string targetDirectory, bool includeUserAliasesOnly, List<string> expectedReturn)
[TestCaseSource(nameof(sourceLists))]
public void TestOnNormalOutput(string targetDirectory, bool includeUserAliasesOnly, List<string> 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();
}
Expand All @@ -45,10 +44,10 @@ public void TestOnError(string codeOwnerPath)
Assert.AreEqual(1, Program.Main(codeOwnerPath, "sdk"));
}

private void TestExpectResult(List<string> expectReturn, string output)
private static void TestExpectResult(List<string> expectReturn, string output)
{
CodeOwnerEntry codeOwnerEntry = JsonSerializer.Deserialize<CodeOwnerEntry>(output);
List<string> actualReturn = codeOwnerEntry.Owners;
List<string> actualReturn = codeOwnerEntry!.Owners;
Assert.AreEqual(expectReturn.Count, actualReturn.Count);
for (int i = 0; i < actualReturn.Count; i++)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.Text.Json;
using Azure.Sdk.Tools.CodeOwnersParser;

Expand All @@ -7,10 +7,10 @@ namespace Azure.Sdk.Tools.RetrieveCodeOwners
/// <summary>
/// The tool command to retrieve code owners.
/// </summary>
public class Program
public static class Program
{
/// <summary>
/// Retrieves codeowners information for specific section of the repo
/// Retrieves CODEOWNERS information for specific section of the repo
/// </summary>
/// <param name="codeOwnerFilePath">The path of CODEOWNERS file in repo</param>
/// <param name="targetDirectory">The directory whose information is to be retrieved</param>
Expand Down
8 changes: 7 additions & 1 deletion tools/code-owners-parser/CodeOwnersParser.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions tools/code-owners-parser/CodeOwnersParser.sln.DotSettings
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=PR/@EntryIndexedValue">PR</s:String>
<s:Boolean x:Key="/Default/UserDictionary/Words/=azconfig/@EntryIndexedValue">True</s:Boolean>
<s:Boolean x:Key="/Default/UserDictionary/Words/=CODEOWNERS/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>
3 changes: 1 addition & 2 deletions tools/code-owners-parser/CodeOwnersParser/CodeOwnerEntry.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
using System;
using System;
using System.Collections.Generic;
using System.Linq;
using System.Net.Http;

namespace Azure.Sdk.Tools.CodeOwnersParser
{
Expand Down
23 changes: 6 additions & 17 deletions tools/code-owners-parser/CodeOwnersParser/CodeOwnersFile.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
using OutputColorizer;
using System;
using System.Collections.Generic;
using System.IO;
Expand All @@ -9,28 +8,23 @@ public static class CodeOwnersFile
{
public static List<CodeOwnerEntry> ParseFile(string filePathOrUrl)
{
string content;
content = FileHelpers.GetFileContents(filePathOrUrl);

string content = FileHelpers.GetFileContents(filePathOrUrl);
return ParseContent(content);
}

public static List<CodeOwnerEntry> ParseContent(string fileContent)
{
List<CodeOwnerEntry> entries = new List<CodeOwnerEntry>();
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();

// 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);

Expand Down Expand Up @@ -93,14 +87,9 @@ public static CodeOwnerEntry FindOwnersForClosestMatch(List<CodeOwnerEntry> 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;
}
}
2 changes: 1 addition & 1 deletion tools/code-owners-parser/CodeOwnersParser/FileHelpers.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System;
using System;
using System.IO;
using System.Net.Http;

Expand Down
8 changes: 5 additions & 3 deletions tools/identity-resolution/Services/GitHubService.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Logging;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
Expand All @@ -14,8 +14,10 @@ namespace Azure.Sdk.Tools.NotificationConfiguration
/// </summary>
public class GitHubService
{
private static HttpClient httpClient = new HttpClient();
private static ConcurrentDictionary<string, List<CodeOwnerEntry>> codeownersFileCache = new ConcurrentDictionary<string, List<CodeOwnerEntry>>();
private static readonly HttpClient httpClient = new HttpClient();

private static readonly ConcurrentDictionary<string, List<CodeOwnerEntry>>
codeownersFileCache = new ConcurrentDictionary<string, List<CodeOwnerEntry>>();

private readonly ILogger<GitHubService> logger;

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<wpf:ResourceDictionary xml:space="preserve" xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:s="clr-namespace:System;assembly=mscorlib" xmlns:ss="urn:shemas-jetbrains-com:settings-storage-xaml" xmlns:wpf="http://schemas.microsoft.com/winfx/2006/xaml/presentation">
<s:String x:Key="/Default/CodeStyle/Naming/CSharpNaming/Abbreviations/=PR/@EntryIndexedValue">PR</s:String>
<s:Boolean x:Key="/Default/UserDictionary/Words/=CODEOWNERS/@EntryIndexedValue">True</s:Boolean></wpf:ResourceDictionary>

0 comments on commit 189316a

Please sign in to comment.