[Fix/Feature] Set Hotfix as release branch by default #2346

dramdrung · 2020-07-02T17:13:35Z

Description

By the definition in the documentation it seems that the Hotfix branches should by default have is-release-branch set to true.
Discussion can be followed at #2336.

This change not only modifies how version in Hotfix branch name is treated but as well how merge messages with such branches are handled.

Related Issue

#2336

Motivation and Context

Version in Hotfix branch names should be taken into consideration when resolving base version by default.
This change enables this behavior with default configuration.

How Has This Been Tested?

The branch was build locally and tested manually.

The default configuration for Hotfix branch has: is-release-branch: true
The version in branch name is taken into consideration when resolving version on hotfix branch

Additionally there were added a few tests and current test scenarios were adjusted to the change.

Checklist:

My code follows the code style of this project.
My change requires a change to the documentation.
I have updated the documentation accordingly.
I have added tests to cover my changes.
All new and existing tests passed.

…e branch by default

asbjornu

The implementation and added tests are great, I'm just torn about the change in functionality. There's quite a few tests that needed change to get this through, making me think we may have a breaking change on our hands here. 🤔

asbjornu · 2020-07-03T00:00:46Z

src/GitVersionCore.Tests/IntegrationTests/HotfixBranchScenarios.cs

@@ -63,7 +63,7 @@ public void CanTakeVersionFromHotfixesBranch()

            // create hotfix branch
            Commands.Checkout(fixture.Repository, fixture.Repository.CreateBranch("hotfixes/1.1.1"));
-            fixture.AssertFullSemver("1.1.0"); // We are still on a tagged commit
+            fixture.AssertFullSemver("1.1.1-beta.1+0"); // Version in branch name takes precedence over Tag if it's greater


Hm. I'm torn and don't really know what to think here. My knee-jerk reaction is that tags should always take precedence over everything else, but then again the entire point with versioned hotfix and release branches is to affect the version number.

I think it makes sense that even if we are on tagged commit on release branch, greater version wins between tag and branch name.
As at first Release branch for X version is just created and optionally some Hotfixes are provided.
But the whole release process on such branch may end with no additional commits.

In the end I just documented what I experienced when I had to update this test case scenario.

Actually I agree with 1.1.1-beta.1+0 as a result for this scenario. If you think about it: You are going to create a hotfix branch from the current tagged version from main (RTM version). Thus your plan is to fix something maybe your CI pipeline has changes or what ever. The point is: This version number gives you an indication that something different happens comparing to the previous version 1.1.0 but no code changes are involved (beta.1+0). ;)

Maybe you have changed from x64 to x32 or used the self-contained flag in dotnet because on your production environment dotnet is not installed.

asbjornu · 2020-07-03T00:01:39Z

src/GitVersionCore.Tests/VersionCalculation/Strategies/MergeMessageBaseVersionStrategyTests.cs

+        [TestCase("Merge branch 'feature-4.6.6' into support-4.6", true)]
+        [TestCase("Merge branch 'Feature-10.10.50'", true)]
+        [TestCase("Merge branch 'Features-10.10.50'", true)]
+        [TestCase("Merge branch 'fix/10.10.50' into hotfix-10.0.0", true)]
+        [TestCase("Merge branch 'feature/0.1.5'", true)]
+        [TestCase("Merge branch 'feature-4.2.2' into support-4.2", true)]


I assume these test cases broke when the branches were named hotfix* and not feature*?

Yes, every test case scenario which is changed in this PR was failing due to the change.

dramdrung · 2020-07-03T06:12:25Z

Yes, my first thought after running the tests were the same.
It seems that with previous change it was probably intended mainly for hotfix branch names to not be taken into consideration, as there were a few test case scenarios which expected such behavior.

I don't really know what to think of it as well.
On the one hand I would expect for Hotfixes to behave like release branches in some cases by default.
But in the end it may be better to just provide appropriate documentation to previous change.

asbjornu · 2020-07-31T23:15:25Z

I think we'll target this for version 6, since it introduces breaking changes.

asbjornu · 2022-03-10T11:23:37Z

main is now primed for v6. Can you please rebase and solve conflicts so we can merge this, @dramdrung?

HHobeck · 2022-09-24T11:43:21Z

I have taken a look into the source code and see that we have two IVersionStrategy implementations in the current version 5.10.3 of GitVersion which are using the IsReleaseBranch property to determine the version. It is on the one hand the VersionInBranchNameVersionStrategy and on the other hand the MergeMessageVersionStrategy class. So far so good.

I'm just wondering why we are not separating this two aspects and make it configurable. For instance we can introduce a new property with the name VersionInBranchNamePattern="{Major}.{Minor}.{Patch}^" or VersionInBranchNamePattern="{Major}.{Minor}^" to specify explicit how the version can be determined.

This has the following advantages:

If some one want's to configure the pattern how the version is encoded in the branch name then he can do it (see [Bug] Wrong version from the release branche [Feature] Wrong version from the release branche #2925)
The aspect getting the version from the branch name and the aspect getting the version from other release branches are separated and can be configured.

asbjornu · 2022-10-12T14:46:48Z

I'm just wondering why we are not separating this two aspects and make it configurable. For instance we can introduce a new property with the name VersionInBranchNamePattern="{Major}.{Minor}.{Patch}^" or VersionInBranchNamePattern="{Major}.{Minor}^" to specify explicit how the version can be determined.

Sure, that's even something we could introduce in v5 since it's a new configuration property.

HHobeck · 2023-02-22T18:01:50Z

Hi @dramdrung. Please rebase your PR with the main branch and give me feedback when you are done.

HHobeck · 2023-02-28T15:54:50Z

Has been resolved with PR #2336 without the need of specifing IsRelease to true for Hotfix branches.

dramdrung added 3 commits June 30, 2020 23:51

Treat Hotfix branches as Release branches by default

5266408

Addition and adjustment to Test Case Scenarios to Hotfix being releas…

e77270c

…e branch by default

Update of default config in documentation

d73aaf0

dramdrung mentioned this pull request Jul 2, 2020

[Bug] Hotfix branch name not taken as version source #2336

Closed

Fix whitespace formatting

d653673

asbjornu reviewed Jul 3, 2020

View reviewed changes

asbjornu added this to the 6.0.0 milestone Jul 31, 2020

Merge branch 'master' into feature/SetHotfixAsReleaseBranch

f940058

Base automatically changed from master to main January 31, 2021 12:46

asbjornu added the breaking change label Mar 9, 2022

HHobeck removed this from the 6.x milestone Feb 21, 2023

HHobeck closed this Feb 28, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix/Feature] Set Hotfix as release branch by default #2346

[Fix/Feature] Set Hotfix as release branch by default #2346

dramdrung commented Jul 2, 2020

asbjornu left a comment

asbjornu Jul 3, 2020

dramdrung Jul 3, 2020

HHobeck Sep 24, 2022 •

edited

Loading

HHobeck Sep 24, 2022

asbjornu Jul 3, 2020

dramdrung Jul 3, 2020

dramdrung commented Jul 3, 2020

asbjornu commented Jul 31, 2020

asbjornu commented Mar 10, 2022

HHobeck commented Sep 24, 2022

asbjornu commented Oct 12, 2022

HHobeck commented Feb 22, 2023

HHobeck commented Feb 28, 2023

[Fix/Feature] Set Hotfix as release branch by default #2346

[Fix/Feature] Set Hotfix as release branch by default #2346

Conversation

dramdrung commented Jul 2, 2020

Description

Related Issue

Motivation and Context

How Has This Been Tested?

Checklist:

asbjornu left a comment

Choose a reason for hiding this comment

asbjornu Jul 3, 2020

Choose a reason for hiding this comment

dramdrung Jul 3, 2020

Choose a reason for hiding this comment

HHobeck Sep 24, 2022 • edited Loading

Choose a reason for hiding this comment

HHobeck Sep 24, 2022

Choose a reason for hiding this comment

asbjornu Jul 3, 2020

Choose a reason for hiding this comment

dramdrung Jul 3, 2020

Choose a reason for hiding this comment

dramdrung commented Jul 3, 2020

asbjornu commented Jul 31, 2020

asbjornu commented Mar 10, 2022

HHobeck commented Sep 24, 2022

asbjornu commented Oct 12, 2022

HHobeck commented Feb 22, 2023

HHobeck commented Feb 28, 2023

HHobeck Sep 24, 2022 •

edited

Loading