From 472f7151a596ee603256c8e9112665b81fd0ecf7 Mon Sep 17 00:00:00 2001 From: Max Date: Mon, 20 May 2019 11:30:27 +0200 Subject: [PATCH] Configurable auto deletion of branch after merge. (#774) * Delete branch after merge. * Work in progress. * Use configured value for Gitlab. * Changed assignment order. * Fixed build. * Work in progress * Removed obsolete test. * Tightened tests. * Updated docs. * Local github repo support (#771) (#12) * Merge branch 'master' of https://github.com/NuKeeperDotNet/NuKeeper * github explanation * remove bitbucket * update commands * set favicon * Add github local support * fix comment * update tests * update references * Local github repo support (#771) (#13) * Merge branch 'master' of https://github.com/NuKeeperDotNet/NuKeeper * github explanation * remove bitbucket * update commands * set favicon * Add github local support * fix comment * update tests * update references * Auto deletion boolean default to true. * Include Bitbucket. Updated docs. * Corrected merge fault. --- .../PullRequestRequestTests.cs | 4 +-- .../Configuration/FileSettingsReaderTests.cs | 11 +++++-- .../CollaborationModels/PullRequestRequest.cs | 4 ++- .../Configuration/BranchSettings.cs | 2 ++ .../Configuration/FileSettings.cs | 1 + NuKeeper.AzureDevOps/AzureDevopsPlatform.cs | 2 +- NuKeeper.BitBucket/BitbucketPlatform.cs | 2 +- NuKeeper.Gitlab/GitlabPlatform.cs | 2 +- NuKeeper.Tests/Commands/GlobalCommandTests.cs | 2 +- .../Commands/OrganisationCommandTests.cs | 2 ++ .../Commands/RepositoryCommandTests.cs | 3 +- .../Commands/CollaborationPlatformCommand.cs | 33 +++++++++++++++++++ NuKeeper/Engine/Packages/PackageUpdater.cs | 2 +- site/content/basics/configuration.md | 4 ++- 14 files changed, 60 insertions(+), 14 deletions(-) diff --git a/NuKeeper.Abstractions.Tests/CollaborationModels/PullRequestRequestTests.cs b/NuKeeper.Abstractions.Tests/CollaborationModels/PullRequestRequestTests.cs index f5ee080ee..ea8289d91 100644 --- a/NuKeeper.Abstractions.Tests/CollaborationModels/PullRequestRequestTests.cs +++ b/NuKeeper.Abstractions.Tests/CollaborationModels/PullRequestRequestTests.cs @@ -9,8 +9,8 @@ public class PullRequestRequestTests [Test] public void ReplacesRemotesWhenCreatingPullRequestRequestObject() { - var pr = new PullRequestRequest("head", "title", "origin/master"); - var pr2 = new PullRequestRequest("head", "title", "master"); + var pr = new PullRequestRequest("head", "title", "origin/master", true); + var pr2 = new PullRequestRequest("head", "title", "master", true); Assert.That(pr.BaseRef, Is.EqualTo("master")); Assert.That(pr2.BaseRef, Is.EqualTo("master")); diff --git a/NuKeeper.Abstractions.Tests/Configuration/FileSettingsReaderTests.cs b/NuKeeper.Abstractions.Tests/Configuration/FileSettingsReaderTests.cs index 7fd099a16..5b1756775 100644 --- a/NuKeeper.Abstractions.Tests/Configuration/FileSettingsReaderTests.cs +++ b/NuKeeper.Abstractions.Tests/Configuration/FileSettingsReaderTests.cs @@ -41,6 +41,7 @@ public void MissingFileReturnsNoSettings() Assert.That(data.LogDestination, Is.Null); Assert.That(data.Platform, Is.Null); Assert.That(data.BranchNamePrefix, Is.Null); + Assert.That(data.DeleteBranchAfterMerge, Is.Null); } [Test] @@ -71,6 +72,7 @@ public void EmptyConfigReturnsNoSettings() Assert.That(data.LogDestination, Is.Null); Assert.That(data.Platform, Is.Null); Assert.That(data.BranchNamePrefix, Is.Null); + Assert.That(data.DeleteBranchAfterMerge, Is.Null); } private const string FullFileData = @"{ @@ -93,7 +95,8 @@ public void EmptyConfigReturnsNoSettings() ""OutputDestination"": ""Console"", ""OutputFileName"" : ""out_42.txt"", ""LogDestination"" : ""file"", - ""Platform"" : ""Bitbucket"" + ""Platform"" : ""Bitbucket"", + ""DeleteBranchAfterMerge"": ""true"" }"; [Test] @@ -115,6 +118,7 @@ public void PopulatedConfigReturnsAllStringSettings() Assert.That(data.LogFile, Is.EqualTo("somefile.log")); Assert.That(data.OutputFileName, Is.EqualTo("out_42.txt")); Assert.That(data.BranchNamePrefix, Is.EqualTo("nukeeper")); + Assert.That(data.DeleteBranchAfterMerge, Is.EqualTo(true)); } [Test] @@ -179,7 +183,8 @@ public void ConfigKeysAreCaseInsensitive() ""MAXrepo"":3, ""vErBoSiTy"": ""Q"", ""CHANGE"": ""PATCH"", - ""bRanCHNamEPREfiX"": ""nukeeper"" + ""bRanCHNamEPREfiX"": ""nukeeper"", + ""deLeTEBranCHafTERMerge"": ""true"" }"; var path = MakeTestFile(configData); @@ -201,6 +206,7 @@ public void ConfigKeysAreCaseInsensitive() Assert.That(data.Verbosity, Is.EqualTo(LogLevel.Quiet)); Assert.That(data.Change, Is.EqualTo(VersionChange.Patch)); Assert.That(data.BranchNamePrefix, Is.EqualTo("nukeeper")); + Assert.That(data.DeleteBranchAfterMerge, Is.EqualTo(true)); } [Test] @@ -223,7 +229,6 @@ public void ExtraKeysAreIgnored() Assert.That(data.Api, Is.EqualTo("http://api.com")); } - private static string MakeTestFile(string contents) { var folder = TemporaryFolder(); diff --git a/NuKeeper.Abstractions/CollaborationModels/PullRequestRequest.cs b/NuKeeper.Abstractions/CollaborationModels/PullRequestRequest.cs index 8dd487d12..31c12e478 100644 --- a/NuKeeper.Abstractions/CollaborationModels/PullRequestRequest.cs +++ b/NuKeeper.Abstractions/CollaborationModels/PullRequestRequest.cs @@ -2,10 +2,11 @@ namespace NuKeeper.Abstractions.CollaborationModels { public class PullRequestRequest { - public PullRequestRequest(string head, string title, string baseRef) + public PullRequestRequest(string head, string title, string baseRef, bool deleteBranchAfterMerge) { Head = head; Title = title; + DeleteBranchAfterMerge = deleteBranchAfterMerge; //This can be a remote that has been passed in, this happens when run locally against a targetbranch that is remote BaseRef = baseRef.Replace("origin/", string.Empty); @@ -15,5 +16,6 @@ public PullRequestRequest(string head, string title, string baseRef) public string Title { get; } public string BaseRef { get; } public string Body { get; set; } + public bool DeleteBranchAfterMerge { get; set; } } } diff --git a/NuKeeper.Abstractions/Configuration/BranchSettings.cs b/NuKeeper.Abstractions/Configuration/BranchSettings.cs index 0557e6daf..413f9a426 100644 --- a/NuKeeper.Abstractions/Configuration/BranchSettings.cs +++ b/NuKeeper.Abstractions/Configuration/BranchSettings.cs @@ -3,5 +3,7 @@ namespace NuKeeper.Abstractions.Configuration public class BranchSettings { public string BranchNamePrefix { get; set; } + + public bool DeleteBranchAfterMerge { get; set; } } } diff --git a/NuKeeper.Abstractions/Configuration/FileSettings.cs b/NuKeeper.Abstractions/Configuration/FileSettings.cs index a6c6e5ddf..9dde5ffe8 100644 --- a/NuKeeper.Abstractions/Configuration/FileSettings.cs +++ b/NuKeeper.Abstractions/Configuration/FileSettings.cs @@ -40,6 +40,7 @@ public class FileSettings public Platform? Platform { get; set; } public string BranchNamePrefix { get; set; } + public bool? DeleteBranchAfterMerge { get; set; } public static FileSettings Empty() { diff --git a/NuKeeper.AzureDevOps/AzureDevopsPlatform.cs b/NuKeeper.AzureDevOps/AzureDevopsPlatform.cs index 88095ea36..6b1758d25 100644 --- a/NuKeeper.AzureDevOps/AzureDevopsPlatform.cs +++ b/NuKeeper.AzureDevOps/AzureDevopsPlatform.cs @@ -47,7 +47,7 @@ public async Task OpenPullRequest(ForkData target, PullRequestRequest request, I targetRefName = $"refs/heads/{request.BaseRef}", completionOptions = new GitPullRequestCompletionOptions { - deleteSourceBranch = true + deleteSourceBranch = request.DeleteBranchAfterMerge } }; diff --git a/NuKeeper.BitBucket/BitbucketPlatform.cs b/NuKeeper.BitBucket/BitbucketPlatform.cs index 57741c1e1..936db6dfc 100644 --- a/NuKeeper.BitBucket/BitbucketPlatform.cs +++ b/NuKeeper.BitBucket/BitbucketPlatform.cs @@ -61,7 +61,7 @@ public async Task OpenPullRequest(ForkData target, PullRequestRequest request, I } }, description = request.Body, - close_source_branch = true + close_source_branch = request.DeleteBranchAfterMerge }; await _client.CreatePullRequest(req, target.Owner, repo.name); diff --git a/NuKeeper.Gitlab/GitlabPlatform.cs b/NuKeeper.Gitlab/GitlabPlatform.cs index 890a8a181..b16c0a595 100644 --- a/NuKeeper.Gitlab/GitlabPlatform.cs +++ b/NuKeeper.Gitlab/GitlabPlatform.cs @@ -49,7 +49,7 @@ public async Task OpenPullRequest(ForkData target, PullRequestRequest request, I Description = request.Body, TargetBranch = request.BaseRef, Id = $"{projectName}/{repositoryName}", - RemoveSourceBranch = true + RemoveSourceBranch = request.DeleteBranchAfterMerge }; await _client.OpenMergeRequest(projectName, repositoryName, mergeRequest); diff --git a/NuKeeper.Tests/Commands/GlobalCommandTests.cs b/NuKeeper.Tests/Commands/GlobalCommandTests.cs index 00a812dce..83884f998 100644 --- a/NuKeeper.Tests/Commands/GlobalCommandTests.cs +++ b/NuKeeper.Tests/Commands/GlobalCommandTests.cs @@ -130,13 +130,13 @@ public async Task EmptyFileResultsInDefaultSettings() Assert.That(settings.UserSettings.OutputFormat, Is.EqualTo(OutputFormat.Text)); Assert.That(settings.BranchSettings.BranchNamePrefix, Is.Null); + Assert.That(settings.BranchSettings.DeleteBranchAfterMerge, Is.EqualTo(true)); Assert.That(settings.SourceControlServerSettings.Scope, Is.EqualTo(ServerScope.Global)); Assert.That(settings.SourceControlServerSettings.IncludeRepos, Is.Null); Assert.That(settings.SourceControlServerSettings.ExcludeRepos, Is.Null); } - [Test] public async Task WillReadApiFromFile() { diff --git a/NuKeeper.Tests/Commands/OrganisationCommandTests.cs b/NuKeeper.Tests/Commands/OrganisationCommandTests.cs index 85661bc57..ce2c071d1 100644 --- a/NuKeeper.Tests/Commands/OrganisationCommandTests.cs +++ b/NuKeeper.Tests/Commands/OrganisationCommandTests.cs @@ -113,6 +113,8 @@ public async Task EmptyFileResultsInDefaultSettings() Assert.That(settings.UserSettings.MaxRepositoriesChanged, Is.EqualTo(10)); + Assert.That(settings.BranchSettings.DeleteBranchAfterMerge, Is.EqualTo(true)); + Assert.That(settings.SourceControlServerSettings.IncludeRepos, Is.Null); Assert.That(settings.SourceControlServerSettings.ExcludeRepos, Is.Null); } diff --git a/NuKeeper.Tests/Commands/RepositoryCommandTests.cs b/NuKeeper.Tests/Commands/RepositoryCommandTests.cs index 9bed6894a..4e570a806 100644 --- a/NuKeeper.Tests/Commands/RepositoryCommandTests.cs +++ b/NuKeeper.Tests/Commands/RepositoryCommandTests.cs @@ -1,7 +1,6 @@ using System; using System.Collections.Generic; using System.Reflection; -using System.Text.RegularExpressions; using System.Threading.Tasks; using NSubstitute; using NuKeeper.Abstractions.CollaborationModels; @@ -13,7 +12,6 @@ using NuKeeper.Collaboration; using NuKeeper.Commands; using NuKeeper.Engine; -using NuKeeper.Git; using NuKeeper.GitHub; using NuKeeper.Inspection.Files; using NuKeeper.Inspection.Logging; @@ -223,6 +221,7 @@ public async Task EmptyFileResultsInDefaultSettings() Assert.That(settings.BranchSettings, Is.Not.Null); Assert.That(settings.BranchSettings.BranchNamePrefix, Is.Null); + Assert.That(settings.BranchSettings.DeleteBranchAfterMerge, Is.EqualTo(true)); Assert.That(settings.SourceControlServerSettings.IncludeRepos, Is.Null); Assert.That(settings.SourceControlServerSettings.ExcludeRepos, Is.Null); diff --git a/NuKeeper/Commands/CollaborationPlatformCommand.cs b/NuKeeper/Commands/CollaborationPlatformCommand.cs index 57daa7ba6..dc0cec734 100644 --- a/NuKeeper/Commands/CollaborationPlatformCommand.cs +++ b/NuKeeper/Commands/CollaborationPlatformCommand.cs @@ -46,6 +46,10 @@ internal abstract class CollaborationPlatformCommand : CommandBase Description = "Sets the collaboration platform type. By default this is inferred from the Url.")] public Platform? Platform { get; set; } + [Option(CommandOptionType.NoValue, ShortName = "", LongName = "deletebranchaftermerge", + Description = "Deletes branch created by NuKeeper after merge. Defaults to false.")] + public bool? DeleteBranchAfterMerge { get; set; } + protected CollaborationPlatformCommand(ICollaborationEngine engine, IConfigureLogger logger, IFileSettingsCache fileSettingsCache, ICollaborationFactory collaborationFactory) : base(logger, fileSettingsCache) @@ -108,6 +112,12 @@ protected override ValidationResult PopulateSettings(SettingsContainer settings) settings.SourceControlServerSettings.Labels = Concat.FirstPopulatedList(Label, fileSettings.Label, defaultLabels); + var deleteBranchAfterMergeValid = PopulateDeleteBranchAfterMerge(settings); + if (!deleteBranchAfterMergeValid.IsSuccess) + { + return deleteBranchAfterMergeValid; + } + return ValidationResult.Success; } @@ -116,5 +126,28 @@ protected override async Task Run(SettingsContainer settings) await _engine.Run(settings); return 0; } + + private ValidationResult PopulateDeleteBranchAfterMerge( + SettingsContainer settings) + { + var fileSettings = FileSettingsCache.GetSettings(); + + if (!Platform.HasValue) + { + settings.BranchSettings.DeleteBranchAfterMerge = true; + return ValidationResult.Success; + } + + if (Platform != Abstractions.CollaborationPlatform.Platform.AzureDevOps + && Platform != Abstractions.CollaborationPlatform.Platform.GitLab + && Platform != Abstractions.CollaborationPlatform.Platform.Bitbucket) + { + return ValidationResult.Failure( + $"Deletion of source branch after merge is currently only available for Azure DevOps, Gitlab and Bitbucket."); + } + + settings.BranchSettings.DeleteBranchAfterMerge = Concat.FirstValue(DeleteBranchAfterMerge, fileSettings.DeleteBranchAfterMerge, true); + return ValidationResult.Success; + } } } diff --git a/NuKeeper/Engine/Packages/PackageUpdater.cs b/NuKeeper/Engine/Packages/PackageUpdater.cs index 101480109..a3a03f2bc 100644 --- a/NuKeeper/Engine/Packages/PackageUpdater.cs +++ b/NuKeeper/Engine/Packages/PackageUpdater.cs @@ -97,7 +97,7 @@ private async Task MakeUpdatePullRequests( qualifiedBranch = repository.Push.Owner + ":" + branchWithChanges; } - var pullRequestRequest = new PullRequestRequest(qualifiedBranch, title, repository.DefaultBranch) { Body = body }; + var pullRequestRequest = new PullRequestRequest(qualifiedBranch, title, repository.DefaultBranch, settings.BranchSettings.DeleteBranchAfterMerge) { Body = body }; await _collaborationFactory.CollaborationPlatform.OpenPullRequest(repository.Pull, pullRequestRequest, settings.SourceControlServerSettings.Labels); diff --git a/site/content/basics/configuration.md b/site/content/basics/configuration.md index 28a1209a6..546f8bd49 100644 --- a/site/content/basics/configuration.md +++ b/site/content/basics/configuration.md @@ -34,6 +34,7 @@ title: "Configuration" | excluderepos | | `org`, `global` | _null_ | | | | | | | branchnameprefix | | `repo`, `org`, `global`, `update`| _null_ | +| deletebranchaftermerge | | _all_ | true | * *age* The minimum package age. In order to not consume packages immediately after they are released, exclude updates that do not meet a minimum age. The default is 7 days. This age is the duration between the published date of the selected package update and now. A value can be expressed in command options as an integer and a unit suffix, @@ -68,9 +69,10 @@ Examples: `0` = zero, `12h` = 12 hours, `3d` = 3 days, `2w` = two weeks. * *maxpackageupdates* The maximum number of package updates to apply. In `repo`,`org` and `global` commands, this limits the number of updates per repository. If the `--consolidate` flag is used, these wll be consolidated into one Pull Request. If not, then there will be one Pull Request per update applied. In the `update` command, The default value is 1. When changed, multiple updates can be applied in a single `update` run, up to this number. * *maxrepo* The maximum number of repositories to change. Used in Organisation and Global mode. * *consolidate* Consolidate updates into a single pull request, instead of the default of 1 pull request per package update applied. -* *platform* One of `GitHub`, `AzureDevOps`, `Bitbucket`, `BitbucketLocal`. Determines which kind of source control api will be used. This is typicaly infered from the api url structure, but since this does not always work, it can be specified here if neccessary. +* *platform* One of `GitHub`, `AzureDevOps`, `Bitbucket`, `BitbucketLocal`, `Gitlab`, `Gitea`. Determines which kind of source control api will be used. This is typicaly infered from the api url structure, but since this does not always work, it can be specified here if neccessary. * *includerepos* A regex to filter repositories by name. Only consider repositories where the name matches this regex pattern. Used in Organisation and Global mode. * *excluderepos* A regex to filter repositories by name. Do not consider repositories where the name matches this regex pattern. Used in Organisation and Global mode. * *branchnameprefix* A prefix that gets added to branch name that NuKeeper creates. Allows you to put those branches in hierarchy (E.G. 'nukeeper/'). +* *deletebranchaftermerge* Specifies whether a branch should be automatically deleted or not once the branch has been merged. Currently only works with `Platform` equal to `AzureDevOps`, `Gitlab` or `Bitbucket`.