Integrate code review comments of reviewer asbjornu and arturcic.

HHobeck · arturcic · commit 613ee7add72f · 2022-10-04T17:25:01.000+02:00
diff --git a/src/GitVersion.Core.Tests/Helpers/TestConfigurationBuilder.cs b/src/GitVersion.Core.Tests/Helpers/TestConfigurationBuilder.cs
@@ -4,9 +4,9 @@
 
 namespace GitVersion.Core.Tests.Helpers;
 
-public sealed class ConfigBuilder
+public sealed class TestConfigurationBuilder
 {
-    public static ConfigBuilder New => new();
+    public static TestConfigurationBuilder New => new();
 
     private string? nextVerson;
     private VersioningMode? versioningMode;
@@ -19,87 +19,87 @@ public sealed class ConfigBuilder
     private readonly Dictionary<string, string?> tagDictionary = new();
     private IgnoreConfig? ignoreConfig;
 
-    private ConfigBuilder()
+    private TestConfigurationBuilder()
     {
         withoutAnyTrackMergeTargets = false;
         increment = IncrementStrategy.Inherit;
         versioningMode = VersioningMode.ContinuousDelivery;
     }
 
-    public ConfigBuilder WithNextVersion(string? value)
+    public TestConfigurationBuilder WithNextVersion(string? value)
     {
         nextVerson = value;
         return this;
     }
 
-    public ConfigBuilder WithVersioningMode(VersioningMode value)
+    public TestConfigurationBuilder WithVersioningMode(VersioningMode value)
     {
         versioningMode = value;
         return this;
     }
 
-    public ConfigBuilder WithoutVersioningMode()
+    public TestConfigurationBuilder WithoutVersioningMode()
     {
         versioningMode = null;
         return this;
     }
 
-    public ConfigBuilder WithVersioningMode(string branch, VersioningMode value)
+    public TestConfigurationBuilder WithVersioningMode(string branch, VersioningMode value)
     {
         versioningModeDictionary[branch] = value;
         return this;
     }
 
-    public ConfigBuilder WithoutVersioningMode(string branch)
+    public TestConfigurationBuilder WithoutVersioningMode(string branch)
     {
         versioningModeDictionary[branch] = null;
         return this;
     }
 
-    public ConfigBuilder WithTrackMergeTarget(string branch, bool value)
+    public TestConfigurationBuilder WithTrackMergeTarget(string branch, bool value)
     {
         trackMergeTargetsDictionary[branch] = value;
         return this;
     }
 
-    public ConfigBuilder WithoutAnyTrackMergeTargets()
+    public TestConfigurationBuilder WithoutAnyTrackMergeTargets()
     {
         withoutAnyTrackMergeTargets = true;
         trackMergeTargetsDictionary.Clear();
         return this;
     }
 
-    public ConfigBuilder WithPreventIncrementOfMergedBranchVersion(string branch, bool value)
+    public TestConfigurationBuilder WithPreventIncrementOfMergedBranchVersion(string branch, bool value)
     {
         preventIncrementOfMergedBranchVersionDictionary[branch] = value;
         return this;
     }
 
-    public ConfigBuilder WithIncrement(IncrementStrategy? value)
+    public TestConfigurationBuilder WithIncrement(IncrementStrategy? value)
     {
         increment = value;
         return this;
     }
 
-    public ConfigBuilder WithIncrement(string branch, IncrementStrategy value)
+    public TestConfigurationBuilder WithIncrement(string branch, IncrementStrategy value)
     {
         incrementDictionary[branch] = value;
         return this;
     }
 
-    public ConfigBuilder WithoutTag(string branch)
+    public TestConfigurationBuilder WithoutTag(string branch)
     {
         tagDictionary[branch] = null;
         return this;
     }
 
-    public ConfigBuilder WithTag(string branch, string value)
+    public TestConfigurationBuilder WithTag(string branch, string value)
     {
         tagDictionary[branch] = value;
         return this;
     }
 
-    public ConfigBuilder WithIgnoreConfig(IgnoreConfig value)
+    public TestConfigurationBuilder WithIgnoreConfig(IgnoreConfig value)
     {
         ignoreConfig = value;
         return this;
diff --git a/src/GitVersion.Core.Tests/VersionCalculation/NextVersionCalculatorTests.cs b/src/GitVersion.Core.Tests/VersionCalculation/NextVersionCalculatorTests.cs
@@ -22,7 +22,7 @@ public void ShouldIncrementVersionBasedOnConfig()
         contextBuilder
             .OverrideServices(services =>
             {
-                var configuration = ConfigBuilder.New.Build();
+                var configuration = TestConfigurationBuilder.New.Build();
                 var branchMock = GitToolsTestingExtensions.CreateMockBranch("main", GitToolsTestingExtensions.CreateMockCommit());
                 var effectiveConfiguration = new EffectiveConfiguration(configuration, configuration.GetBranchConfiguration("main"));
                 var effectiveBranchConfiguration = new EffectiveBranchConfiguration(branchMock, effectiveConfiguration);
@@ -54,7 +54,7 @@ public void DoesNotIncrementWhenBaseVersionSaysNotTo()
         contextBuilder
             .OverrideServices(services =>
             {
-                var configuration = ConfigBuilder.New.Build();
+                var configuration = TestConfigurationBuilder.New.Build();
                 var branchMock = GitToolsTestingExtensions.CreateMockBranch("main", GitToolsTestingExtensions.CreateMockCommit());
                 var effectiveConfiguration = new EffectiveConfiguration(configuration, configuration.GetBranchConfiguration("main"));
                 var effectiveBranchConfiguration = new EffectiveBranchConfiguration(branchMock, effectiveConfiguration);
@@ -86,7 +86,7 @@ public void AppliesBranchPreReleaseTag()
         contextBuilder
             .OverrideServices(services =>
             {
-                var configuration = ConfigBuilder.New.Build();
+                var configuration = TestConfigurationBuilder.New.Build();
                 var branchMock = GitToolsTestingExtensions.CreateMockBranch("develop", GitToolsTestingExtensions.CreateMockCommit());
                 var effectiveConfiguration = new EffectiveConfiguration(configuration, configuration.GetBranchConfiguration("develop"));
                 var effectiveBranchConfiguration = new EffectiveBranchConfiguration(branchMock, effectiveConfiguration);
diff --git a/src/GitVersion.Core.Tests/VersionCalculation/Strategies/VersionInBranchNameBaseVersionStrategyTests.cs b/src/GitVersion.Core.Tests/VersionCalculation/Strategies/VersionInBranchNameBaseVersionStrategyTests.cs
@@ -23,7 +23,7 @@ public void CanTakeVersionFromNameOfReleaseBranch(string branchName, string expe
 
         var gitRepository = fixture.Repository.ToGitRepository();
         var strategy = GetVersionStrategy(fixture.RepositoryPath, gitRepository, branchName);
-        var configuration = ConfigBuilder.New.Build();
+        var configuration = TestConfigurationBuilder.New.Build();
         var branchConfiguration = configuration.GetBranchConfiguration(branchName);
         var effectiveConfiguration = new EffectiveConfiguration(configuration, branchConfiguration);
         var baseVersion = strategy.GetBaseVersions(new(gitRepository.FindBranch(branchName)!, effectiveConfiguration)).Single();
@@ -43,7 +43,7 @@ public void ShouldNotTakeVersionFromNameOfNonReleaseBranch(string branchName)
 
         var gitRepository = fixture.Repository.ToGitRepository();
         var strategy = GetVersionStrategy(fixture.RepositoryPath, gitRepository, branchName);
-        var configuration = ConfigBuilder.New.Build();
+        var configuration = TestConfigurationBuilder.New.Build();
         var branchConfiguration = configuration.GetBranchConfiguration(branchName);
         var effectiveConfiguration = new EffectiveConfiguration(configuration, branchConfiguration);
         var baseVersions = strategy.GetBaseVersions(new(gitRepository.FindBranch(branchName)!, effectiveConfiguration));
@@ -66,7 +66,7 @@ public void CanTakeVersionFromNameOfConfiguredReleaseBranch(string branchName, s
         var gitRepository = fixture.Repository.ToGitRepository();
         var strategy = GetVersionStrategy(fixture.RepositoryPath, gitRepository, branchName, config);
 
-        var configuration = ConfigBuilder.New.Build();
+        var configuration = TestConfigurationBuilder.New.Build();
         var branchConfiguration = configuration.GetBranchConfiguration(branchName);
         var effectiveConfiguration = new EffectiveConfiguration(configuration, branchConfiguration);
         var baseVersion = strategy.GetBaseVersions(new(gitRepository.FindBranch(branchName)!, effectiveConfiguration)).Single();
@@ -89,7 +89,7 @@ public void CanTakeVersionFromNameOfRemoteReleaseBranch(string branchName, strin
         var gitRepository = fixture.Repository.ToGitRepository();
         var strategy = GetVersionStrategy(fixture.RepositoryPath, gitRepository, branchName);
 
-        var configuration = ConfigBuilder.New.Build();
+        var configuration = TestConfigurationBuilder.New.Build();
         var branchConfiguration = configuration.GetBranchConfiguration(branchName);
         var effectiveConfiguration = new EffectiveConfiguration(configuration, branchConfiguration);
         var baseVersion = strategy.GetBaseVersions(new(gitRepository.FindBranch(branchName)!, effectiveConfiguration)).Single();
diff --git a/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs b/src/GitVersion.Core/Configuration/BranchConfigurationCalculator.cs
@@ -34,7 +34,7 @@ private BranchConfig GetBranchConfigurationInternal(int recursions, IBranch targ
             throw new InfiniteLoopProtectionException($"Inherited branch configuration caused {recursions} recursions. Aborting!");
         }
 
-        var matchingBranches = configuration.FindConfigurationForBranch(targetBranch.Name.WithoutRemote);
+        var matchingBranches = configuration.ForBranch(targetBranch);
 
         if (matchingBranches == null)
         {
diff --git a/src/GitVersion.Core/Configuration/ConfigExtensions.cs b/src/GitVersion.Core/Configuration/ConfigExtensions.cs
@@ -12,7 +12,7 @@ public static BranchConfig GetBranchConfiguration(this Config configuration, IBr
 
     public static BranchConfig GetBranchConfiguration(this Config configuration, string branchName)
     {
-        var branchConfiguration = FindConfigurationForBranch(configuration, branchName);
+        var branchConfiguration = ForBranch(configuration, branchName);
         if (branchConfiguration is null)
         {
             branchConfiguration = GetUnknownBranchConfiguration(configuration);
@@ -54,9 +54,12 @@ public static BranchConfig GetFallbackBranchConfiguration(this Config configurat
         return result;
     }
 
-    internal static BranchConfig? FindConfigurationForBranch(this Config config, string branchName)
+    internal static BranchConfig? ForBranch(this Config configuration, IBranch branch)
+    => GetBranchConfiguration(configuration, branch.NotNull().Name.WithoutRemote);
+
+    internal static BranchConfig? ForBranch(this Config configuration, string branchName)
     {
-        var matches = config.Branches
+        var matches = configuration.Branches
             .Where(b => b.Value?.Regex != null && Regex.IsMatch(branchName, b.Value.Regex, RegexOptions.IgnoreCase))
             .ToArray();
 
diff --git a/src/GitVersion.Core/Core/RepositoryStore.cs b/src/GitVersion.Core/Core/RepositoryStore.cs
@@ -155,9 +155,9 @@ public IEnumerable<IBranch> GetBranchesForCommit(ICommit commit)
     public IEnumerable<IBranch> GetExcludedInheritBranches(Config configuration)
         => this.repository.Branches.Where(b =>
         {
-            var branchConfig = configuration.FindConfigurationForBranch(b.Name.WithoutRemote);
+            var branchConfiguration = configuration.ForBranch(b);
 
-            return branchConfig == null || branchConfig.Increment == IncrementStrategy.Inherit;
+            return branchConfiguration == null || branchConfiguration.Increment == IncrementStrategy.Inherit;
         }).ToList();
 
     public IEnumerable<IBranch> GetReleaseBranches(IEnumerable<KeyValuePair<string, BranchConfig>> releaseBranchConfig)

Original file line number	Diff line number	Diff line change
`@@ -4,9 +4,9 @@`
`4`	`4`
`5`	`5`	`namespace GitVersion.Core.Tests.Helpers;`
`6`	`6`
`7`		`-public sealed class ConfigBuilder`
	`7`	`+public sealed class TestConfigurationBuilder`
`8`	`8`	`{`
`9`		`- public static ConfigBuilder New => new();`
	`9`	`+ public static TestConfigurationBuilder New => new();`
`10`	`10`
`11`	`11`	`private string? nextVerson;`
`12`	`12`	`private VersioningMode? versioningMode;`
`@@ -19,87 +19,87 @@ public sealed class ConfigBuilder`
`19`	`19`	`private readonly Dictionary<string, string?> tagDictionary = new();`
`20`	`20`	`private IgnoreConfig? ignoreConfig;`
`21`	`21`
`22`		`- private ConfigBuilder()`
	`22`	`+ private TestConfigurationBuilder()`
`23`	`23`	`{`
`24`	`24`	`withoutAnyTrackMergeTargets = false;`
`25`	`25`	`increment = IncrementStrategy.Inherit;`
`26`	`26`	`versioningMode = VersioningMode.ContinuousDelivery;`
`27`	`27`	`}`
`28`	`28`
`29`		`- public ConfigBuilder WithNextVersion(string? value)`
	`29`	`+ public TestConfigurationBuilder WithNextVersion(string? value)`
`30`	`30`	`{`
`31`	`31`	`nextVerson = value;`
`32`	`32`	`return this;`
`33`	`33`	`}`
`34`	`34`
`35`		`- public ConfigBuilder WithVersioningMode(VersioningMode value)`
	`35`	`+ public TestConfigurationBuilder WithVersioningMode(VersioningMode value)`
`36`	`36`	`{`
`37`	`37`	`versioningMode = value;`
`38`	`38`	`return this;`
`39`	`39`	`}`
`40`	`40`
`41`		`- public ConfigBuilder WithoutVersioningMode()`
	`41`	`+ public TestConfigurationBuilder WithoutVersioningMode()`
`42`	`42`	`{`
`43`	`43`	`versioningMode = null;`
`44`	`44`	`return this;`
`45`	`45`	`}`
`46`	`46`
`47`		`- public ConfigBuilder WithVersioningMode(string branch, VersioningMode value)`
	`47`	`+ public TestConfigurationBuilder WithVersioningMode(string branch, VersioningMode value)`
`48`	`48`	`{`
`49`	`49`	`versioningModeDictionary[branch] = value;`
`50`	`50`	`return this;`
`51`	`51`	`}`
`52`	`52`
`53`		`- public ConfigBuilder WithoutVersioningMode(string branch)`
	`53`	`+ public TestConfigurationBuilder WithoutVersioningMode(string branch)`
`54`	`54`	`{`
`55`	`55`	`versioningModeDictionary[branch] = null;`
`56`	`56`	`return this;`
`57`	`57`	`}`
`58`	`58`
`59`		`- public ConfigBuilder WithTrackMergeTarget(string branch, bool value)`
	`59`	`+ public TestConfigurationBuilder WithTrackMergeTarget(string branch, bool value)`
`60`	`60`	`{`
`61`	`61`	`trackMergeTargetsDictionary[branch] = value;`
`62`	`62`	`return this;`
`63`	`63`	`}`
`64`	`64`
`65`		`- public ConfigBuilder WithoutAnyTrackMergeTargets()`
	`65`	`+ public TestConfigurationBuilder WithoutAnyTrackMergeTargets()`
`66`	`66`	`{`
`67`	`67`	`withoutAnyTrackMergeTargets = true;`
`68`	`68`	`trackMergeTargetsDictionary.Clear();`
`69`	`69`	`return this;`
`70`	`70`	`}`
`71`	`71`
`72`		`- public ConfigBuilder WithPreventIncrementOfMergedBranchVersion(string branch, bool value)`
	`72`	`+ public TestConfigurationBuilder WithPreventIncrementOfMergedBranchVersion(string branch, bool value)`
`73`	`73`	`{`
`74`	`74`	`preventIncrementOfMergedBranchVersionDictionary[branch] = value;`
`75`	`75`	`return this;`
`76`	`76`	`}`
`77`	`77`
`78`		`- public ConfigBuilder WithIncrement(IncrementStrategy? value)`
	`78`	`+ public TestConfigurationBuilder WithIncrement(IncrementStrategy? value)`
`79`	`79`	`{`
`80`	`80`	`increment = value;`
`81`	`81`	`return this;`
`82`	`82`	`}`
`83`	`83`
`84`		`- public ConfigBuilder WithIncrement(string branch, IncrementStrategy value)`
	`84`	`+ public TestConfigurationBuilder WithIncrement(string branch, IncrementStrategy value)`
`85`	`85`	`{`
`86`	`86`	`incrementDictionary[branch] = value;`
`87`	`87`	`return this;`
`88`	`88`	`}`
`89`	`89`
`90`		`- public ConfigBuilder WithoutTag(string branch)`
	`90`	`+ public TestConfigurationBuilder WithoutTag(string branch)`
`91`	`91`	`{`
`92`	`92`	`tagDictionary[branch] = null;`
`93`	`93`	`return this;`
`94`	`94`	`}`
`95`	`95`
`96`		`- public ConfigBuilder WithTag(string branch, string value)`
	`96`	`+ public TestConfigurationBuilder WithTag(string branch, string value)`
`97`	`97`	`{`
`98`	`98`	`tagDictionary[branch] = value;`
`99`	`99`	`return this;`
`100`	`100`	`}`
`101`	`101`
`102`		`- public ConfigBuilder WithIgnoreConfig(IgnoreConfig value)`
	`102`	`+ public TestConfigurationBuilder WithIgnoreConfig(IgnoreConfig value)`
`103`	`103`	`{`
`104`	`104`	`ignoreConfig = value;`
`105`	`105`	`return this;`
Original file line number	Diff line number	Diff line change
`@@ -34,7 +34,7 @@ private BranchConfig GetBranchConfigurationInternal(int recursions, IBranch targ`
`34`	`34`	`throw new InfiniteLoopProtectionException($"Inherited branch configuration caused {recursions} recursions. Aborting!");`
`35`	`35`	`}`
`36`	`36`
`37`		`- var matchingBranches = configuration.FindConfigurationForBranch(targetBranch.Name.WithoutRemote);`
	`37`	`+ var matchingBranches = configuration.ForBranch(targetBranch);`
`38`	`38`
`39`	`39`	`if (matchingBranches == null)`
`40`	`40`	`{`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ public static BranchConfig GetBranchConfiguration(this Config configuration, IBr`
`12`	`12`
`13`	`13`	`public static BranchConfig GetBranchConfiguration(this Config configuration, string branchName)`
`14`	`14`	`{`
`15`		`- var branchConfiguration = FindConfigurationForBranch(configuration, branchName);`
	`15`	`+ var branchConfiguration = ForBranch(configuration, branchName);`
`16`	`16`	`if (branchConfiguration is null)`
`17`	`17`	`{`
`18`	`18`	`branchConfiguration = GetUnknownBranchConfiguration(configuration);`
`@@ -54,9 +54,12 @@ public static BranchConfig GetFallbackBranchConfiguration(this Config configurat`
`54`	`54`	`return result;`
`55`	`55`	`}`
`56`	`56`
`57`		`- internal static BranchConfig? FindConfigurationForBranch(this Config config, string branchName)`
	`57`	`+ internal static BranchConfig? ForBranch(this Config configuration, IBranch branch)`
	`58`	`+ => GetBranchConfiguration(configuration, branch.NotNull().Name.WithoutRemote);`
	`59`	`+`
	`60`	`+ internal static BranchConfig? ForBranch(this Config configuration, string branchName)`
`58`	`61`	`{`
`59`		`- var matches = config.Branches`
	`62`	`+ var matches = configuration.Branches`
`60`	`63`	`.Where(b => b.Value?.Regex != null && Regex.IsMatch(branchName, b.Value.Regex, RegexOptions.IgnoreCase))`
`61`	`64`	`.ToArray();`
`62`	`65`