You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I reckoned if I changed that to master the generated test scenarios would behave accordingly, initializing and working with "master" instead of "main". But no, that constant is very sparingly used; many many magic strings all over and trying to make the tests behave quickly became a very noisy PR; so I just narrowed in on the absolute minimum so I could back this part out.
In the meantime I also encountered this (there's a whole suite of keys here):
The concept of BranchKey aligns to the standard sections the GitVersion.yml anticipates.
Throughout the test suite both "classes" of constant are passed around as strings and it gets really confusing very quickly which is correct and which won't work in various places in the code.
Could I suggest introducing a clear split, making the ConfigurationBranchKey a type safe concept that can be used to tidy up the string passing methods and parameters while leaving the Tester free to choose whatever actual branch names they want for commits to the repo.
Something like this perhaps (yes I've gone quite far down the rabbit hole already but because the magic strings are pervasive and widespread the PR will be big (~130 files) but very focused and imho fairly easy to delta). Test coverage is obviously good because this is all about tests, barely impacts the core functional code at all. High confidence that I can fit this in without breaking or significantly altering any existing tests (incl. TestCaseAttributes, Branches[]/SourceBranches, BranchMetaData, Custom, Unknown & Other).
namespace GitVersion.Configuration;
public class BranchConfigurationKey : IEquatable<BranchConfigurationKey>
{
[Obsolete("Use MainBranchKey instead.")]
public static readonly BranchConfigurationKey Master = new(ConfigurationConstants.MasterBranchKey);
public static readonly BranchConfigurationKey Main = new(ConfigurationConstants.MainBranchKey);
public static readonly BranchConfigurationKey Develop = new(ConfigurationConstants.DevelopBranchKey);
public static readonly BranchConfigurationKey Release = new(ConfigurationConstants.ReleaseBranchKey);
public static readonly BranchConfigurationKey Feature = new(ConfigurationConstants.FeatureBranchKey);
public static readonly BranchConfigurationKey PullRequest = new(ConfigurationConstants.PullRequestBranchKey);
public static readonly BranchConfigurationKey Hotfix = new(ConfigurationConstants.HotfixBranchKey);
public static readonly BranchConfigurationKey Support = new(ConfigurationConstants.SupportBranchKey);
public string Value { get; }
protected BranchConfigurationKey(string value)
{
if (string.IsNullOrEmpty(value))
throw new ArgumentException("Value cannot be null or empty.", nameof(value));
Value = value;
}
public override string ToString() => Value;
}
public class CustomBranchConfigurationKey : BranchConfigurationKey
{
private static readonly HashSet<string> ReservedKeys = new(StringComparer.Ordinal)
{
ConfigurationConstants.MasterBranchKey,
ConfigurationConstants.MainBranchKey,
ConfigurationConstants.DevelopBranchKey,
ConfigurationConstants.ReleaseBranchKey,
ConfigurationConstants.FeatureBranchKey,
ConfigurationConstants.PullRequestBranchKey,
ConfigurationConstants.HotfixBranchKey,
ConfigurationConstants.SupportBranchKey,
};
public CustomBranchConfigurationKey(string value) : base(value)
{
if (value is null)
throw new ArgumentNullException(nameof(value), "Custom branch key cannot be null.");
if (ReservedKeys.Contains(value))
throw new ArgumentException($"'{value}' is a reserved branch configuration key and cannot be used for custom branches.", nameof(value));
}
}
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
-
We still use master in almost all repos and I've been struggling to get a working config in v6 so I started looking at the test suite.
Quite soon I came across many "main" magic strings but also this:
GitVersion/src/GitVersion.Core.Tests/Helpers/TestBase.cs
Line 10 in f8a95ab
I reckoned if I changed that to master the generated test scenarios would behave accordingly, initializing and working with "master" instead of "main". But no, that constant is very sparingly used; many many magic strings all over and trying to make the tests behave quickly became a very noisy PR; so I just narrowed in on the absolute minimum so I could back this part out.
In the meantime I also encountered this (there's a whole suite of keys here):
GitVersion/src/GitVersion.Core/Configuration/ConfigurationConstants.cs
Line 31 in f8a95ab
The concept of BranchKey aligns to the standard sections the GitVersion.yml anticipates.
Throughout the test suite both "classes" of constant are passed around as strings and it gets really confusing very quickly which is correct and which won't work in various places in the code.
Could I suggest introducing a clear split, making the ConfigurationBranchKey a type safe concept that can be used to tidy up the string passing methods and parameters while leaving the Tester free to choose whatever actual branch names they want for commits to the repo.
Something like this perhaps (yes I've gone quite far down the rabbit hole already but because the magic strings are pervasive and widespread the PR will be big (~130 files) but very focused and imho fairly easy to delta). Test coverage is obviously good because this is all about tests, barely impacts the core functional code at all. High confidence that I can fit this in without breaking or significantly altering any existing tests (incl. TestCaseAttributes, Branches[]/SourceBranches, BranchMetaData, Custom, Unknown & Other).
Beta Was this translation helpful? Give feedback.
All reactions