PR Review Commit

Will squash once review is complete

src/GitReleaseManager.Cli/Program.cs

@@ -202,17 +202,15 @@ private static void RegisterVcsProvider(BaseVcsOptions vcsOptions, IServiceColle
             Log.Information("Using {Provider} as VCS Provider", vcsOptions.Provider);
             if (vcsOptions.Provider == VcsProvider.GitLab)
             {
-                var gitlabClient = new GitLabClient("https://gitlab.com", vcsOptions.Token);
                 serviceCollection
-                    .AddSingleton<IVcsProvider, GitLabProvider>()
-                    .AddSingleton<IGitLabClient>(gitlabClient);
+                    .AddSingleton<IGitLabClient>((_) => new GitLabClient("https://gitlab.com", vcsOptions.Token))
+                    .AddSingleton<IVcsProvider, GitLabProvider>();
             }
             else
             {
                 // default to Github
-                var gitHubClient = new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(vcsOptions.Token) };
                 serviceCollection
-                    .AddSingleton<IGitHubClient>(gitHubClient)
+                    .AddSingleton<IGitHubClient>((_) => new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(vcsOptions.Token) })
                     .AddSingleton<IVcsProvider, GitHubProvider>();
             }
         }

src/GitReleaseManager.Core/Commands/AddAssetsCommand.cs

@@ -21,7 +21,7 @@ public async Task<int> ExecuteAsync(AddAssetSubOptions options)
 
             if (vcsOptions?.Provider == Model.VcsProvider.GitLab)
             {
-                _logger.Error("The addasset command is currently not supported when targetting GitLab.");
+                _logger.Error("The 'addasset' command is currently not supported when targeting GitLab.");
                 return 1;
             }
 

src/GitReleaseManager.Core/Commands/ExportCommand.cs

@@ -18,13 +18,13 @@ public ExportCommand(IVcsService vcsService, ILogger logger)
 
         public async Task<int> ExecuteAsync(ExportSubOptions options)
         {
-            if (string.IsNullOrEmpty(options.TagName))
+            if (string.IsNullOrWhiteSpace(options.TagName))
             {
-                _logger.Information("Exporting all releases");
+                _logger.Information("Exporting all releases.");
             }
             else
             {
-                _logger.Information("Exporting release {TagName}", options.TagName);
+                _logger.Information("Exporting release {TagName}.", options.TagName);
             }
 
             var releasesContent = await _vcsService.ExportReleasesAsync(options.RepositoryOwner, options.RepositoryName, options.TagName, options.SkipPrereleases).ConfigureAwait(false);

src/GitReleaseManager.Core/Commands/LabelCommand.cs

@@ -21,7 +21,7 @@ public async Task<int> ExecuteAsync(LabelSubOptions options)
 
             if (vcsOptions?.Provider == Model.VcsProvider.GitLab)
             {
-                _logger.Error("The label command is currently not supported when targetting GitLab.");
+                _logger.Error("The label command is currently not supported when targeting GitLab.");
                 return 1;
             }
 

src/GitReleaseManager.Core/Extensions/MilestoneExtensions.cs

@@ -17,13 +17,13 @@ public static Version Version(this Octokit.Milestone ver)
             var nameWithoutPrerelease = ver.Title.Split('-')[0];
             if (nameWithoutPrerelease.StartsWith("v", StringComparison.OrdinalIgnoreCase))
             {
-                _logger.Debug("Removing version prefix from {Name}", ver.Title);
+                _logger.Debug("Removing version prefix from {Name}.", ver.Title);
                 nameWithoutPrerelease = nameWithoutPrerelease.Remove(0, 1);
             }
 
             if (!System.Version.TryParse(nameWithoutPrerelease, out Version parsedVersion))
             {
-                _logger.Warning("No valid version was found on {Title}", ver.Title);
+                _logger.Warning("No valid version was found on {Title}.", ver.Title);
                 return new Version(0, 0);
             }
 

src/GitReleaseManager.Core/MappingProfiles/GitHubProfile.cs

@@ -1,3 +1,4 @@
+using System;
 using AutoMapper;
 using GitReleaseManager.Core.Extensions;
 
@@ -10,7 +11,7 @@ public GitHubProfile()
             CreateMap<Octokit.Issue, Model.Issue>()
                 .ForMember(dest => dest.PublicNumber, act => act.MapFrom(src => src.Number))
                 .ForMember(dest => dest.InternalNumber, act => act.MapFrom(src => src.Id))
-                .ForMember(dest => dest.IsPullRequest, act => act.MapFrom(src => src.HtmlUrl.Contains("/pull/")))
+                .ForMember(dest => dest.IsPullRequest, act => act.MapFrom(src => src.HtmlUrl.IndexOf("/pull/", StringComparison.OrdinalIgnoreCase) >= 0))
                 .ReverseMap();
             CreateMap<Model.IssueComment, Octokit.IssueComment>().ReverseMap();
             CreateMap<Model.ItemState, Octokit.ItemState>().ReverseMap();

src/GitReleaseManager.Core/Provider/GitLabProvider.cs

@@ -49,20 +49,21 @@ public Task DeleteAssetAsync(string owner, string repository, ReleaseAsset asset
 
         public Task UploadAssetAsync(Release release, ReleaseAssetUpload releaseAssetUpload)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
-                _logger.Warning("Uploading of assets is not currently supported when targetting GitLab.");
+                _logger.Warning("Uploading of assets is not currently supported when targeting GitLab.");
+                return Task.CompletedTask;
             });
         }
 
         public Task<int> GetCommitsCountAsync(string owner, string repository, string @base, string head)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 // TODO: This is waiting on a PR being merged...
                 // https://github.com/ubisoft/NGitLab/pull/444
                 // Once it is, we might be able to implement what is necessary here.
-                return 0;
+                return Task.FromResult(0);
             });
         }
 
@@ -79,7 +80,7 @@ public string GetCommitsUrl(string owner, string repository, string head, string
 
         public Task CreateIssueCommentAsync(string owner, string repository, Issue issue, string comment)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var projectId = GetGitLabProjectId(owner, repository);
 
@@ -105,12 +106,14 @@ public Task CreateIssueCommentAsync(string owner, string repository, Issue issue
 
                     issueNotesClient.Create(issueComment);
                 }
+
+                return Task.CompletedTask;
             });
         }
 
         public Task<IEnumerable<Issue>> GetIssuesAsync(string owner, string repository, Milestone milestone, ItemStateFilter itemStateFilter = ItemStateFilter.All)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var issuesClient = _gitLabClient.Issues;
 
@@ -150,13 +153,13 @@ public Task<IEnumerable<Issue>> GetIssuesAsync(string owner, string repository,
                 issuesAndMergeRequests.AddRange(_mapper.Map<IEnumerable<Issue>>(issues));
                 issuesAndMergeRequests.AddRange(_mapper.Map<IEnumerable<Issue>>(mergeRequests));
 
-                return issuesAndMergeRequests.AsEnumerable();
+                return Task.FromResult(issuesAndMergeRequests.AsEnumerable());
             });
         }
 
         public Task<IEnumerable<IssueComment>> GetIssueCommentsAsync(string owner, string repository, Issue issue)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 IEnumerable<IssueComment> issueComments = Enumerable.Empty<IssueComment>();
                 var projectId = GetGitLabProjectId(owner, repository);
@@ -175,7 +178,7 @@ public Task<IEnumerable<IssueComment>> GetIssueCommentsAsync(string owner, strin
                     issueComments = _mapper.Map<IEnumerable<IssueComment>>(comments);
                 }
 
-                return issueComments;
+                return Task.FromResult(issueComments);
             });
         }
 
@@ -221,7 +224,7 @@ public Task<Milestone> GetMilestoneAsync(string owner, string repository, string
 
         public Task<IEnumerable<Milestone>> GetMilestonesAsync(string owner, string repository, ItemStateFilter itemStateFilter = ItemStateFilter.All)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var query = new MilestoneQuery();
 
@@ -244,13 +247,13 @@ public Task<IEnumerable<Milestone>> GetMilestonesAsync(string owner, string repo
                     mappedMilestone.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/milestones/{2}#tab-issues", owner, repository, mappedMilestone.PublicNumber);
                 }
 
-                return mappedMilestones;
+                return Task.FromResult(mappedMilestones);
             });
         }
 
         public Task SetMilestoneStateAsync(string owner, string repository, Milestone milestone, ItemState itemState)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var mileStoneClient = _gitLabClient.GetMilestone(GetGitLabProjectId(owner, repository));
 
@@ -262,12 +265,14 @@ public Task SetMilestoneStateAsync(string owner, string repository, Milestone mi
                 {
                     mileStoneClient.Close(milestone.InternalNumber);
                 }
+
+                return Task.CompletedTask;
             });
         }
 
         public Task<Release> CreateReleaseAsync(string owner, string repository, Release release)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository));
 
@@ -280,23 +285,24 @@ public Task<Release> CreateReleaseAsync(string owner, string repository, Release
                     mappedRelease.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/releases/{2}", owner, repository, release.TagName);
                 }
 
-                return mappedRelease;
+                return Task.FromResult(mappedRelease);
             });
         }
 
         public Task DeleteReleaseAsync(string owner, string repository, Release release)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository));
 
                 releaseClient.Delete(release.TagName);
+                return Task.CompletedTask;
             });
         }
 
         public Task<Release> GetReleaseAsync(string owner, string repository, string tagName)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository));
 
@@ -310,7 +316,7 @@ public Task<Release> GetReleaseAsync(string owner, string repository, string tag
                     mappedRelease.HtmlUrl = string.Format(CultureInfo.InvariantCulture, "https://gitlab.com/{0}/{1}/-/releases/{2}", owner, repository, tagName);
                 }
 
-                return mappedRelease;
+                return Task.FromResult(mappedRelease);
             });
         }
 
@@ -326,7 +332,7 @@ public Task<IEnumerable<Release>> GetReleasesAsync(string owner, string reposito
 
         public Task PublishReleaseAsync(string owner, string repository, string tagName, Release release)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository));
 
@@ -337,18 +343,18 @@ public Task PublishReleaseAsync(string owner, string repository, string tagName,
                 };
 
                 releaseClient.Update(update);
+                return Task.CompletedTask;
             });
         }
 
         public Task UpdateReleaseAsync(string owner, string repository, Release release)
         {
-            return ExecuteAsync(async () =>
+            return ExecuteAsync(() =>
             {
                 var releaseClient = _gitLabClient.GetReleases(GetGitLabProjectId(owner, repository));
 
                 var update = new ReleaseUpdate
                 {
-
                     Description = release.Body,
                     ReleasedAt = release.Draft ? DateTime.UtcNow.AddYears(1) : DateTime.UtcNow,
                     Name = release.Name,
@@ -357,6 +363,7 @@ public Task UpdateReleaseAsync(string owner, string repository, Release release)
                 };
 
                 releaseClient.Update(update);
+                return Task.CompletedTask;
             });
         }
 
@@ -370,7 +377,7 @@ public RateLimit GetRateLimit()
 
         public string GetMilestoneQueryString()
         {
-            return "sort=due_date_desc&state=closed";
+            return "state=closed";
         }
 
         public string GetIssueType(Issue issue)
@@ -398,14 +405,10 @@ private async Task ExecuteAsync(Func<Task> action)
             {
                 await action().ConfigureAwait(false);
             }
-            ////catch (Octokit.ForbiddenException ex)
-            ////{
-            ////    throw new ForbiddenException(ex.Message, ex);
-            ////}
-            ////catch (Octokit.NotFoundException ex)
-            ////{
-            ////    throw new NotFoundException(ex.Message, ex);
-            ////}
+            catch (AggregateException ae)
+            {
+                throw new ApiException(ae.Message, ae);
+            }
             catch (Exception ex) when (!(ex is NotFoundException))
             {
                 throw new ApiException(ex.Message, ex);
@@ -418,14 +421,10 @@ private async Task<T> ExecuteAsync<T>(Func<Task<T>> action)
             {
                 return await action().ConfigureAwait(false);
             }
-            ////catch (Octokit.ForbiddenException ex)
-            ////{
-            ////    throw new ForbiddenException(ex.Message, ex);
-            ////}
-            ////catch (Octokit.NotFoundException ex)
-            ////{
-            ////    throw new NotFoundException(ex.Message, ex);
-            ////}
+            catch (AggregateException ae)
+            {
+                throw new ApiException(ae.Message, ae);
+            }
             catch (Exception ex) when (!(ex is NotFoundException))
             {
                 throw new ApiException(ex.Message, ex);

src/GitReleaseManager.Core/ReleaseNotes/ReleaseNotesBuilder.cs

@@ -60,7 +60,7 @@ public async Task<string> BuildReleaseNotesAsync(string user, string repository,
 
             if (issues.Count == 0)
             {
-                var logMessage = string.Format(CultureInfo.InvariantCulture, "No closed issues have been found for milestone {0}, or all assigned issues are meant to be excluded from release notes, aborting creation of release.", _milestoneTitle);
+                var logMessage = string.Format(CultureInfo.CurrentCulture, "No closed issues have been found for milestone {0}, or all assigned issues are meant to be excluded from release notes, aborting release creation.", _milestoneTitle);
                 throw new InvalidOperationException(logMessage);
             }
 

src/GitReleaseManager.Core/VcsService.cs

@@ -142,7 +142,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam
                     {
                         if (!File.Exists(asset))
                         {
-                            var message = string.Format(CultureInfo.InvariantCulture, "Requested asset to be uploaded doesn't exist: {0}", asset);
+                            var message = string.Format(CultureInfo.CurrentCulture, "The requested asset to be uploaded doesn't exist: {0}", asset);
                             throw new FileNotFoundException(message);
                         }
 
@@ -155,7 +155,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam
 
                             if (_vcsProvider is GitLabProvider)
                             {
-                                _logger.Error("Deleting of assets is not currently supported when targetting GitLab.");
+                                _logger.Error("Deleting assets is not currently supported when targeting GitLab.");
                             }
                             else
                             {
@@ -186,7 +186,7 @@ private async Task AddAssetsAsync(string owner, string repository, string tagNam
                         if (!release.Body.Contains(_configuration.Create.ShaSectionHeading))
                         {
                             _logger.Debug("Creating SHA section header");
-                            stringBuilder.AppendLine(string.Format(CultureInfo.InvariantCulture, "### {0}", _configuration.Create.ShaSectionHeading));
+                            stringBuilder.AppendFormat(CultureInfo.InvariantCulture, "### {0}", _configuration.Create.ShaSectionHeading).AppendLine();
                         }
 
                         foreach (var asset in assets)
@@ -405,7 +405,7 @@ private async Task AddIssueCommentsAsync(string owner, string repository, Milest
                 }
                 catch (ForbiddenException)
                 {
-                    _logger.Error("Unable to add comment to issue #{IssueNumber}. Insufficient permissions.", issue.PublicNumber);
+                    _logger.Error("Unable to add a comment to issue #{IssueNumber}. Insufficient permissions.", issue.PublicNumber);
                     break;
                 }
             }

src/GitReleaseManager.IntegrationTests/GitHubProviderIntegrationTests.cs

@@ -6,7 +6,6 @@
 using GitReleaseManager.Core.Provider;
 using NUnit.Framework;
 using Octokit;
-using Serilog;
 using Shouldly;
 using Issue = GitReleaseManager.Core.Model.Issue;
 using Milestone = GitReleaseManager.Core.Model.Milestone;
@@ -24,7 +23,6 @@ public class GitHubProviderIntegrationTests
         private GitHubProvider _gitHubProvider;
         private IGitHubClient _gitHubClient;
         private IMapper _mapper;
-        private ILogger _logger;
 
         private string _token;
         private string _releaseBaseTag;
@@ -43,7 +41,6 @@ public void OneTimeSetUp()
             }
 
             _mapper = AutoMapperConfiguration.Configure();
-            _logger = new LoggerConfiguration().WriteTo.Console().CreateLogger();
             _gitHubClient = new GitHubClient(new ProductHeaderValue("GitReleaseManager")) { Credentials = new Credentials(_token) };
             _gitHubProvider = new GitHubProvider(_gitHubClient, _mapper);
         }