Fix bug: VersionInBranchNameVersionStrategy only considers the release branch #3455
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,7 @@ | |
|
|
||
| <ItemGroup> | ||
| <Using Include="System.Collections"/> | ||
| <Using Include="System.Text"/> | ||
| </ItemGroup> | ||
|
|
||
| </Project> | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| namespace GitVersion.Helpers; | ||
|
|
||
| public static class EncodingHelper | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,3 @@ | ||
| namespace GitVersion.Infrastructure; | ||
|
|
||
| public interface IFileSystem | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,3 @@ | ||
| using GitVersion.Helpers; | ||
|
|
||
| namespace GitVersion.Infrastructure; | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -60,7 +60,7 @@ public void CanTakeVersionFromHotfixesBranch() | |
|
|
||
| // create hotfix branch | ||
| Commands.Checkout(fixture.Repository, fixture.Repository.CreateBranch("hotfixes/1.1.1")); | ||
| fixture.AssertFullSemver("1.1.1+0"); | ||
| fixture.Repository.MakeACommit(); | ||
|
|
||
| fixture.AssertFullSemver("1.1.1-beta.1+1"); | ||
|
|
@@ -78,44 +78,44 @@ public void PatchOlderReleaseExample() | |
| r.MakeATaggedCommit("2.0.0"); | ||
| }); | ||
| // Merge hotfix branch to support | ||
| fixture.Checkout(MainBranch); | ||
| var tag = fixture.Repository.Tags.Single(t => t.FriendlyName == "1.1.0"); | ||
| fixture.Repository.CreateBranch("support-1.1", (LibGit2Sharp.Commit)tag.Target); | ||
| fixture.Checkout("support-1.1"); | ||
| fixture.AssertFullSemver("1.1.0"); | ||
|
|
||
| // create hotfix branch | ||
| fixture.BranchTo("hotfix-1.1.1"); | ||
| fixture.AssertFullSemver("1.1.1+0"); | ||
| fixture.MakeACommit(); | ||
|
|
||
| fixture.AssertFullSemver("1.1.1-beta.1+1"); | ||
| fixture.MakeACommit(); | ||
| fixture.AssertFullSemver("1.1.1-beta.1+2"); | ||
|
|
||
| // Create feature branch off hotfix branch and complete | ||
| fixture.BranchTo("feature/fix"); | ||
| fixture.AssertFullSemver("1.1.1-fix.1+2"); | ||
| fixture.MakeACommit(); | ||
| fixture.AssertFullSemver("1.1.1-fix.1+3"); | ||
|
|
||
| fixture.Repository.CreatePullRequestRef("feature/fix", "hotfix-1.1.1", prNumber: 8, normalise: true); | ||
| fixture.AssertFullSemver("1.1.1-PullRequest8.4"); | ||
| fixture.Checkout("hotfix-1.1.1"); | ||
| fixture.MergeNoFF("feature/fix"); | ||
| fixture.AssertFullSemver("1.1.1-beta.1+4"); | ||
|
|
||
| // Merge hotfix into support branch to complete hotfix | ||
| fixture.Checkout("support-1.1"); | ||
| fixture.MergeNoFF("hotfix-1.1.1"); | ||
| fixture.AssertFullSemver("1.1.1+5"); | ||
| fixture.ApplyTag("1.1.1"); | ||
| fixture.AssertFullSemver("1.1.1"); | ||
|
|
||
| // Verify develop version | ||
| fixture.Checkout("develop"); | ||
| fixture.AssertFullSemver("2.1.0-alpha.1"); | ||
| fixture.MergeNoFF("support-1.1"); | ||
| fixture.AssertFullSemver("2.1.0-alpha.7"); | ||
| } | ||
|
|
||
|
|
@@ -167,7 +167,7 @@ public void FeatureOnHotfixFeatureBranchDeleted() | |
| fixture.Checkout(hotfix451); | ||
| fixture.MergeNoFF(featureBranch); // commit 2 | ||
| fixture.Repository.Branches.Remove(featureBranch); | ||
| fixture.AssertFullSemver("4.5.1-beta.3", configuration); | ||
|
There was a problem hiding this comment. Why did this go from There was a problem hiding this comment. Hmm very good question. I think it has something to do with the logic in GitVersionVariables.cs which you know will be changed in PR #2347. I have checked the source of the resulting base version and it looks okay for me:
There was a problem hiding this comment. But the There was a problem hiding this comment. I have take a look and I think it's correct. If you take a look to the following figure:
then you see that GitVersion uses the highest version (4.5.1). If there are ambiguous results (like in this case) the oldest commit as a source will be taken. Here it is the commit with the identifier bf7a8f7f. Thus if you have this in mind If you don't agree then we need to create a bug or feature issue and change the logic. But this has nothing to do with this PR IMO. |
||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -217,7 +217,8 @@ public void FeatureOnHotfixFeatureBranchNotDeleted() | |
| fixture.MakeACommit("blabla"); // commit 1 | ||
| fixture.Checkout(hotfix451); | ||
| fixture.MergeNoFF(featureBranch); // commit 2 | ||
|
|
||
| fixture.AssertFullSemver("4.5.1-beta.3", configuration); | ||
| } | ||
|
|
||
| [Test] | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we not still on a tagged commit here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm very good question. I think you have found a bug. ;) The same question applies to the following scenario:
Why it yields to 1.1.0 and not to 1.0.0?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is one of those edge-cases that no matter which solution we pick, we are going to make some people unhappy. I'm in the "a tag should always win" camp, but I can definitely see how some people would expect
release/1.1.0to yield1.1.0even though the same commit is tagged1.0.0. I don't think there's a right answer here, we just need to pick one way or the other, stick to it and document it well.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this discussion is related to this issue here: Same version computed on different branches #3453
We need to introduce a new branch related property (like
take-incremented-version) or make the behavior dependent on the usedversion-modeoption and make it configurable. The point is if you are on the hotfix branch you don't want to have properly the tagged version you would like to have the next version1.1.0+0. Other way around if you are on the main branch you would like to have the tagged version1.0.0. In both cases you are on the same commit with different result dependent on what branch you are. If you don't want this behavior neither then you are able to change it. I see the following enum values:TakeAlwaysBaseVersion,TakeTaggedOtherwiseIncrementedVersion,TakeAlwaysIncrementedVersionThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we were to base this on
mode, how would you suggest we do it?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm I think it would be not a good idea to put it on the
version-modeoption because it is very confusing and would lead to issues complaining about that the tag is not recognice correctly.Also if I think about the behavior of real live deplyoment scenarios (trunk-based, continues-deployment, continues-delivery and manually-deployment) this question is totally independent of that. Because it depends on the fact: Do I create a artifact after I have tagged the commit or before. Or do I take a pre-release version and just re-declare it to production version or not.
What is your opinion about that?