-
Notifications
You must be signed in to change notification settings - Fork 648
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Improvement] Separate SemVerSource from CommitCountSource #3041
Comments
Wonder if this would solve #2795 ? |
Hi @asbjornu. I would like to understand the problem and viewed the initial bug #465 and the actual fix for that #478 and tried to create an integration test which reproduce this issue. Could you please take a look and tell me what the expectation is? And also what was generated before this bug fix. [Test]
public void __Just_A_Test__()
{
var configuration = ConfigBuilder.New.Build();
using var fixture = new EmptyRepositoryFixture();
fixture.Repository.MakeATaggedCommit("1.2.0");
fixture.BranchTo("develop");
fixture.MakeACommit(); // one
fixture.MakeACommit(); // two
// ✅ succeeds as expected
fixture.AssertFullSemver("1.3.0-alpha.2", configuration);
fixture.Checkout("main");
fixture.BranchTo("hotfix/1.2.1");
fixture.MakeACommit(); // three
fixture.MakeACommit(); // four
fixture.ApplyTag("1.2.1-beta.1");
// ✅ succeeds as expected
fixture.AssertFullSemver("1.2.1-beta.1", configuration);
fixture.Checkout("main");
fixture.MergeNoFF("hotfix/1.2.1");
fixture.ApplyTag("1.2.1");
// ✅ succeeds as expected
fixture.AssertFullSemver("1.2.1", configuration);
fixture.Checkout("develop");
// ✅ succeeds as expected
fixture.AssertFullSemver("1.3.0-alpha.2", configuration);
fixture.MergeNoFF("hotfix/1.2.1");
// ❌ expected: 1.3.0-alpha.2 or 1.3.0-alpha.3 or 1.3.0-alpha.4 or 1.3.0-alpha.5 ???
fixture.AssertFullSemver("1.3.0-alpha.???", configuration);
} |
What GitVersion did before #478 is impossible for me to answer. I have no idea. 🤷🏼 When it comes to the test you've written, I think |
And this is the reason why the bugfix has been implemented and you want to separate the SemVerSource from CommitCountSource? Means in this example the SemVerSource is 5 and the CommitCountSource is 3? Or what is the your expectation? |
The output of the SemVer variable is {
"AssemblySemFileVer": "1.3.0.0",
"AssemblySemVer": "1.3.0.0",
"BranchName": "develop",
"BuildMetaData": null,
"CommitDate": "2024-01-11",
"CommitsSinceVersionSource": 5,
"EscapedBranchName": "develop",
"FullBuildMetaData": "Branch.develop.Sha.3f7d98e22589faf4352f21b3735f8d83a9c99181",
"FullSemVer": "1.3.0-alpha.5",
"InformationalVersion": "1.3.0-alpha.5+Branch.develop.Sha.3f7d98e22589faf4352f21b3735f8d83a9c99181",
"Major": 1,
"MajorMinorPatch": "1.3.0",
"Minor": 3,
"Patch": 0,
"PreReleaseLabel": "alpha",
"PreReleaseLabelWithDash": "-alpha",
"PreReleaseNumber": 5,
"PreReleaseTag": "alpha.5",
"PreReleaseTagWithDash": "-alpha.5",
"SemVer": "1.3.0-alpha.5",
"Sha": "3f7d98e22589faf4352f21b3735f8d83a9c99181",
"ShortSha": "3f7d98e",
"UncommittedChanges": 0,
"VersionSourceSha": "45530eca118a96112469bde8a632db9310fdae7e",
"WeightedPreReleaseNumber": 5
} Thus everything is good right!? Could you give me an example where the SemVerSource and the CommitCountSource is not the same? It would be good to have clear acceptance criteria for this improvement. |
I think this test illustrates the problem very well: [TestCase("0.3.0-alpha.1+4", false)]
[TestCase("0.3.0-alpha.1+1", true)]
public void VersionSource(string semanticVersion, bool removeReleaseBranchAfterMerging)
{
// Arrange
var configuration = GitFlowConfigurationBuilder.New
.WithBranch("develop", _ => _
.WithVersioningMode(VersioningMode.ManualDeployment)
.WithTrackMergeMessage(false)
).Build();
// Act
using var fixture = new EmptyRepositoryFixture("main");
fixture.MakeATaggedCommit("0.1.0");
fixture.BranchTo("develop");
fixture.MakeACommit();
fixture.BranchTo("release/0.2.0");
fixture.MakeACommit();
fixture.MergeTo("main", removeReleaseBranchAfterMerging);
fixture.ApplyTag("0.2.0");
fixture.MergeTo("develop");
// Assert
fixture.AssertFullSemver(semanticVersion, configuration);
} I'm still not sure how the idea of introducing two new variables with name |
@HHobeck , that's what I ended up reading this issue in hopes of.
@HHobeck, I'd like to use your lovely tool a little differently than you intended -- it's so close, but not quite what I'd wanted to do.
At the moment, GitVersion doesn't return me any data on which to distinguish a tagless commit that bumped the SemVer from a tagless commit that came after the tagless commit that bumped the SemVer. Meaning I can't say, "hey, this is a semver-bumping commit; go ahead and return the suggested version number." |
@kkgthb: Please create a new discussion if it is not related to this issue. Maybe this issue goes in the direction you mentioned: Still I miss a description about the motivation. Why would you tried the build or whatever differently? |
Currently, the term
VersionSource
is used both for calculation of the SemVer and for commit counting. TheVersionSource
output variables all point to the the source used for commit counting, not the source used for the resulting SemVer.Detailed Description
The conflation of commit count source and SemVer source was introduced in #478, with the reasoning given in #465 that to avoid resetting the commit count, the oldest tag is used as the source for commit counting. The oldest tag is however not used as a source for the resulting SemVer, leading to the codebase operating on two different version sources.
Context
The conflation of commit count source and SemVer source is confusing, as #1830 and #2394 is proof of. A way to remove this confusion is to split
VersionSource
into two different concepts:SemVerSource
andCommitCountSource
.Possible Implementation
The entire codebase needs to be refactored and every mention of
VersionSource
needs to be renamed to eitherSemVerSource
orCommitCountSource
depending on its usage.The text was updated successfully, but these errors were encountered: