Comment Health Pass (#129)

* Comment health review update.

* Fix strange diff with client provider

* Fix Validation diff

* Fix Validation diff

build/azure-pipelines.yml

@@ -370,31 +370,6 @@ stages:
         artifactName: MsixBundle_Release
         targetPath: StorePublish
 
-    # Automated localization coming soon
-    # - task: MicrosoftTDBuild.tdbuild-task.tdbuild-task.TouchdownBuildTask@1
-    #   displayName: Download and Use Localization Files
-    #   condition: eq(variables['EnableLocalization'], 'true')
-    #   retryCountOnTaskFailure: 2
-    #   inputs:
-    #     teamId: 71220
-    #     authId: $(TouchdownAppId)
-    #     authKey: $(TouchdownAppKey)
-    #     resourceFilePath: |
-    #       **\en-US\PDP.xml
-    #     localizationTarget: false
-    #     appendRelativeDir: true
-
-    # - task: PowerShell@2
-    #   displayName: Move Loc files one level up
-    #   condition: eq(variables['EnableLocalization'], 'true')
-    #   inputs:
-    #     targetType: inline
-    #     script: >-
-    #       $Files = Get-ChildItem . -R -Filter 'Resources.resw' | ? FullName -Like '*en-US\*\Resources.resw'
-
-    #       $Files | % { Move-Item -Verbose $_.Directory $_.Directory.Parent.Parent -EA:Ignore }
-    #     pwsh: true
-
     - task: MS-RDX-MRO.windows-store-publish-dev.package-task.store-package@2
       displayName: 'Create Staging StoreBroker Package'
       condition: eq(variables['BuildingBranch'], 'staging')

docs/readme.md

@@ -19,6 +19,7 @@ Once you've discussed your proposed feature/fix/etc. with a team member, and you
 - Package new ideas into classes or refactor existing ideas into a class as you extend.
 - When adding new classes/methods/changing existing code: add new unit tests or update the existing tests.
 <!--
+
 ## GitHub Workflow
 
 - Before starting to work on a fix/feature, make sure there is an open issue to track the work.
@@ -33,18 +34,18 @@ Once you've discussed your proposed feature/fix/etc. with a team member, and you
   - mark the issue(s), that the PR solved, with the `Resolution-Fix-Committed` label, remove the `In progress` label and if the issue is assigned to a project, move the item to the `Done` status.
   - don't close the issue if it's a bug in the current released version since users tend to not search for closed issues, we will close the resolved issues when a new version is released.
   - if it's not a code fix that effects the end user, the issue can be closed (for example a fix in the build or a code refactoring and so on).
--->
+    -->
+
 ## Compiling GITServices
 
 ### Compiling Source Code
 
 There are two ways to compile locally.
 
 - Open the Developer Command Prompt for Visual Studio
-- Run `Build` from GITServices's root directory.  You can pass in a list of platforms/configurations
+- Run `Build` from GITServices's root directory. You can pass in a list of platforms/configurations
 - The GITServices MSIX will be in your repo under `AppxPackages\x64\debug`
 
 Alternatively
 
 - Open `GITServices.sln` in Visual Studio, in the `Solutions Configuration` drop-down menu select `Release` or `Debug`, from the `Build` menu choose `Build Solution`.
-

docs/style.md

@@ -1,12 +1,13 @@
 # Coding Style
 
 ## Philosophy
+
 1. If it's inserting something into the existing classes/functions, try to follow the existing style as closely as possible.
 1. If it's brand new code or refactoring a complete class or area of the code, please follow as Modern C# of a style as you can and reference the [.NET Engineering Guidelines](https://github.com/dotnet/aspnetcore/wiki/Engineering-guidelines) as much as you possibly can.
 
 ## Formatting
 
 - We use [`.clang-format`](/.clang-format) style file to enable automatic code formatting. You can [easily format source files from Visual Studio](https://devblogs.microsoft.com/cppblog/clangformat-support-in-visual-studio-2017-15-7-preview-1/). For example, `CTRL+K CTRL+D` formats the current document.
 - If you prefer another text editor or have ClangFormat disabled in Visual Studio, you could invoke [`format_sources`](/codeAnalysis/format_sources.ps1) powershell script from command line. It gets a list of all currently modified files from `git` and invokes clang-format on them.
-Please note that you should also have `clang-format.exe` in `%PATH%` for it to work. The script can infer the path of `clang-format.exe` version which is shipped with Visual Studio at `%VCINSTALLDIR%\Tools\Llvm\bin\`, if you launch it from the *Native Tools Command Prompt for VS*.
-- CI doesn't enforce code formatting yet, since we're gradually applying code formatting to the codebase, but please adhere to our formatting style for any new code.
+  Please note that you should also have `clang-format.exe` in `%PATH%` for it to work. The script can infer the path of `clang-format.exe` version which is shipped with Visual Studio at `%VCINSTALLDIR%\Tools\Llvm\bin\`, if you launch it from the _Native Tools Command Prompt for VS_.
+- CI doesn't enforce code formatting yet, since we're gradually applying code formatting to the codebase, but please adhere to our formatting style for any new code.

src/GithubPlugin/Client/GithubClientProvider.cs

@@ -1,14 +1,13 @@
 // Copyright (c) Microsoft Corporation and Contributors
 // Licensed under the MIT license.
 
-using DevHome.Logging.Helpers;
+using DevHome.Logging.Helpers;
 using GitHubPlugin.DeveloperId;
 using Microsoft.Windows.DevHome.SDK;
 using Octokit;
 
 namespace GitHubPlugin.Client;
 
-// manage all octokit clients per devid session
 public class GitHubClientProvider
 {
     private readonly GitHubClient publicRepoClient;
@@ -38,10 +37,9 @@ public GitHubClientProvider()
         publicRepoClient = new GitHubClient(new ProductHeaderValue(Constants.DEV_HOME_APPLICATION_NAME));
     }
 
-    // look up or create octokit client in memory
     public GitHubClient? GetClient(IDeveloperId devId)
-    {
-        var devIdInternal = DeveloperIdProvider.GetInstance().GetDeveloperIdInternal(devId) ?? throw new ArgumentException(devId.LoginId());
+    {
+        var devIdInternal = DeveloperIdProvider.GetInstance().GetDeveloperIdInternal(devId) ?? throw new ArgumentException(devId.LoginId());
         return devIdInternal.GitHubClient;
     }
 
@@ -54,41 +52,41 @@ public GitHubClient GetClient(string url)
         }
 
         return devIdInternal.GitHubClient;
-    }
-
-    public GitHubClient GetClient()
-    {
-        return publicRepoClient;
-    }
-
-    public async Task<GitHubClient> GetClientForLoggedInDeveloper(bool logRateLimit = false)
-    {
-        var authProvider = DeveloperIdProvider.GetInstance();
-        var devIds = authProvider.GetLoggedInDeveloperIdsInternal();
-        GitHubClient client;
-        if (devIds == null || !devIds.Any())
-        {
-            Log.Logger()?.ReportInfo($"No logged in developer, using public GitHub client.");
-            client = Instance.GetClient();
-        }
-        else
-        {
-            Log.Logger()?.ReportInfo($"Using authenticated user: {devIds.First().LoginId}");
-            client = devIds.First().GitHubClient;
-        }
-
-        if (client == null)
-        {
-            Log.Logger()?.ReportError($"Failed creating GitHubClient.");
-            return client!;
-        }
-
-        if (logRateLimit)
-        {
-            var miscRateLimit = await client.RateLimit.GetRateLimits();
-            Log.Logger()?.ReportInfo($"Rate Limit:  Remaining: {miscRateLimit.Resources.Core.Remaining}  Total: {miscRateLimit.Resources.Core.Limit}  Resets: {miscRateLimit.Resources.Core.Reset.ToStringInvariant()}");
-        }
-
-        return client;
+    }
+
+    public GitHubClient GetClient()
+    {
+        return publicRepoClient;
+    }
+
+    public async Task<GitHubClient> GetClientForLoggedInDeveloper(bool logRateLimit = false)
+    {
+        var authProvider = DeveloperIdProvider.GetInstance();
+        var devIds = authProvider.GetLoggedInDeveloperIdsInternal();
+        GitHubClient client;
+        if (devIds == null || !devIds.Any())
+        {
+            Log.Logger()?.ReportInfo($"No logged in developer, using public GitHub client.");
+            client = Instance.GetClient();
+        }
+        else
+        {
+            Log.Logger()?.ReportInfo($"Using authenticated user: {devIds.First().LoginId}");
+            client = devIds.First().GitHubClient;
+        }
+
+        if (client == null)
+        {
+            Log.Logger()?.ReportError($"Failed creating GitHubClient.");
+            return client!;
+        }
+
+        if (logRateLimit)
+        {
+            var miscRateLimit = await client.RateLimit.GetRateLimits();
+            Log.Logger()?.ReportInfo($"Rate Limit:  Remaining: {miscRateLimit.Resources.Core.Remaining}  Total: {miscRateLimit.Resources.Core.Limit}  Resets: {miscRateLimit.Resources.Core.Reset.ToStringInvariant()}");
+        }
+
+        return client;
     }
 }

src/GithubPlugin/Client/Validation.cs

@@ -1,11 +1,11 @@
-// Copyright (c) Microsoft Corporation and Contributors
-// Licensed under the MIT license.
+// Copyright (c) Microsoft Corporation and Contributors
+// Licensed under the MIT license.
 using GitHubPlugin.DataModel;
 
-namespace GitHubPlugin.Client;
-
-// Validation layer to help parsing github url.
-public static class Validation
+namespace GitHubPlugin.Client;
+
+// Validation layer to help parsing GitHub URL.
+public static class Validation
 {
     private static bool IsValidHttpUri(string uriString, out Uri? uri)
     {
@@ -15,7 +15,7 @@ private static bool IsValidHttpUri(string uriString, out Uri? uri)
 
     public static bool IsValidGitHubURL(Uri uri)
     {
-        // Valid github Uri has three segments.  The first is /
+        // Valid GitHub URL has three segments.  The first is '/'.
         if (uri.Segments.Length < 3 || (!uri.Host.Equals("github.com", StringComparison.OrdinalIgnoreCase) && !uri.Host.Equals("www.github.com", StringComparison.OrdinalIgnoreCase)))
         {
             Log.Logger()?.ReportDebug($"{uri.OriginalString} is not a valid github uri");
@@ -24,22 +24,22 @@ public static bool IsValidGitHubURL(Uri uri)
 
         return true;
     }
-
-    // Ensure it is a GitHub repo url.
-    public static bool IsValidGitHubURL(string url)
+
+    // Ensure it is a GitHub repo URL.
+    public static bool IsValidGitHubURL(string url)
     {
         Uri? parsedUri;
 
         // https://github.com/dotnet/runtime/issues/72632
-        // IsWellFormedUriString returnes false with a github url.
+        // IsWellFormedUriString returns false with a GitHub URL.
         // Above link shows a work around.
         if (!IsValidHttpUri(url, out parsedUri) || url == null || parsedUri == null)
         {
             Log.Logger()?.ReportDebug($"{url} is not a valid http uri");
             return false;
         }
 
-        return IsValidGitHubURL(parsedUri);
+        return IsValidGitHubURL(parsedUri);
     }
 
     public static bool IsValidGitHubIssueQueryURL(string url)
@@ -80,43 +80,43 @@ public static Uri GetUriFromGitHubUrlString(string url)
         return new Uri(RemoveDotGitFromEndOfString(url));
     }
 
-    public static string ParseOwnerFromGitHubURL(string url)
+    public static string ParseOwnerFromGitHubURL(string url)
     {
-        // Check if url string provided as just the repository FullName.
+        // Check if URL string provided as just the repository FullName.
         var fullNameSplit = GetNameAndRepoFromFullName(url);
         if (fullNameSplit is not null)
         {
             return fullNameSplit[0];
         }
 
-        return ParseOwnerFromGitHubURL(GetUriFromGitHubUrlString(url));
+        return ParseOwnerFromGitHubURL(GetUriFromGitHubUrlString(url));
     }
-
-    public static string ParseOwnerFromGitHubURL(Uri url)
+
+    public static string ParseOwnerFromGitHubURL(Uri url)
     {
         // For some reason Segments is returning trailing '/', even though the documentation
         // remarks state that it strips out the separator. This is a fix for that which will
         // work even if/when that issue is fixed.
-        return url.Segments[1].Replace("/", string.Empty);
+        return url.Segments[1].Replace("/", string.Empty);
     }
 
-    public static string ParseRepositoryFromGitHubURL(string url)
+    public static string ParseRepositoryFromGitHubURL(string url)
     {
-        // Check if url string provided as just the repository FullName.
+        // Check if URL string provided as just the repository FullName.
         var fullNameSplit = GetNameAndRepoFromFullName(url);
         if (fullNameSplit is not null)
         {
             return fullNameSplit[1];
         }
 
-        return ParseRepositoryFromGitHubURL(GetUriFromGitHubUrlString(url));
+        return ParseRepositoryFromGitHubURL(GetUriFromGitHubUrlString(url));
     }
-
-    public static string ParseRepositoryFromGitHubURL(Uri url)
+
+    public static string ParseRepositoryFromGitHubURL(Uri url)
     {
-        // Replace .git because Ocktokit does not want .git.
+        // Replace .git because Octokit does not want .git.
         var repoName = url.Segments[2].Replace("/", string.Empty);
-        return RemoveDotGitFromEndOfString(repoName);
+        return RemoveDotGitFromEndOfString(repoName);
     }
 
     public static string ParseIssueQueryFromGitHubURL(string url)
@@ -163,7 +163,7 @@ private static string RemoveDotGitFromEndOfString(string stringWithDotGit)
 
     public static string ParseFullNameFromGitHubURL(string url)
     {
-        // Check if url string provided as just the repository FullName.
+        // Check if URL string provided as just the repository FullName.
         var fullNameSplit = GetNameAndRepoFromFullName(url);
         if (fullNameSplit is not null)
         {
@@ -173,11 +173,11 @@ public static string ParseFullNameFromGitHubURL(string url)
         return ParseFullNameFromGitHubURL(GetUriFromGitHubUrlString(url));
     }
 
-    public static string ParseFullNameFromGitHubURL(Uri url)
+    public static string ParseFullNameFromGitHubURL(Uri url)
     {
-        // Need to account for the presence or absence of a trailing '/' in the segements, and
+        // Need to account for the presence or absence of a trailing '/' in the segments, and
         // ensure there is exactly one slash separator in the full name.
-        return $"{url.Segments[1].Replace("/", string.Empty)}/{url.Segments[2].Replace("/", string.Empty)}";
+        return $"{url.Segments[1].Replace("/", string.Empty)}/{url.Segments[2].Replace("/", string.Empty)}";
     }
 
     // Adds a protocol to a string to allow for protocol-less Uris.
@@ -198,4 +198,4 @@ private static string AddProtocolToString(string s)
 
         return n;
     }
-}
+}

src/GithubPlugin/Configuration/DeveloperOAuthConfiguration.cs

@@ -14,7 +14,7 @@ internal static class DeveloperOAuthConfiguration
     ////                       setx GITHUB_CLIENT_SECRET "Your OAuth App's ClientSecret" /m
 
     // GitHub OAuth Client ID and Secret values should not be checked in. Rather than modifying these values,
-    // setting the environment variables like shown above will persist across branch switches
+    // setting the environment variables like shown above will persist across branch switches.
     internal static readonly string? ClientID = Environment.GetEnvironmentVariable("GITHUB_CLIENT_ID");
 
     internal static readonly string? ClientSecret = Environment.GetEnvironmentVariable("GITHUB_CLIENT_SECRET");

src/GithubPlugin/Configuration/OAuthConfiguration.cs

@@ -24,7 +24,7 @@ public static string GetClientId()
         {
             if (string.IsNullOrEmpty(DeveloperOAuthConfiguration.ClientID))
             {
-                // Throw if neither the Build-time constant or the environment variable is set
+                // Throw if neither the Build-time constant or the environment variable is set.
                 throw new InvalidOperationException("ClientID has not been set.");
             }
 
@@ -42,7 +42,7 @@ public static string GetClientSecret()
         {
             if (string.IsNullOrEmpty(DeveloperOAuthConfiguration.ClientSecret))
             {
-                // Throw if neither the Build-time constant or the environment variable is set
+                // Throw if neither the Build-time constant or the environment variable is set.
                 throw new InvalidOperationException("ClientSecret has not been set.");
             }
 

src/GithubPlugin/DataManager/DataUpdater.cs

@@ -57,7 +57,7 @@ public void Stop()
 
     public override string ToString() => "DataUpdater";
 
-    private bool disposed; // To detect redundant calls
+    private bool disposed; // To detect redundant calls.
 
     protected virtual void Dispose(bool disposing)
     {