GitTools / GitVersion

From git log to SemVer in no time
https://gitversion.net/docs/
MIT License
2.83k stars 648 forks source link

[ISSUE]: Bumping major/breaking via commit message not working as described during 0.x #4184

Open Sharparam opened 2 weeks ago

Sharparam commented 2 weeks ago

Prerequisites

GitVersion package

GitVersion.Tool

GitVersion version

6.0.2+Branch.main.Sha.30211316bc16e481dc440baae39ff904c4fa4966

Operating system

Linux, Windows

What are you seeing?

When bumping the major version (via +semver:breaking in the commit message) during "initial development" phase (0.x.y), GitVersion bumps the normal major component (which is 0 during initial development) to 1 instead of the minor component.

What is expected?

GitVersion should bump the minor version instead during initial development in 0.x versions.

As described in the documentation:

One thing to be aware of: If the current version is an alpha-version (i.e. 0.x.y.), attempting to bump the major version will merely bump the minor (eg from 0.2.0 to 0.3.0 instead of 1.0.0). Once the current version is greater than 1.0.0, bumping the major version works as expected.

(Emphasis mine)

Steps to Reproduce

Commands to show the issue:

mkdir gv-test && cd gv-test && git init
dotnet new tool-manifest
dotnet tool install gitversion.tool
git commit --allow-empty -m "Initial commit"
dotnet gitversion /showvariable SemVer # Outputs 0.0.1-1 as expected
git tag v0.1.0 -a -m ""
dotnet gitversion /showvariable SemVer # Outputs 0.1.0 as expected
git commit --allow-empty -m "Breaking change!\n\n+semver:breaking"
dotnet gitversion /showvariable SemVer # Outputs 1.0.0-1 but should be 0.2.0-1!

RepositoryFixture Test

New test can be found in PR #4185.

Output log or link to your CI build (if appropriate).

N/A
Sharparam commented 2 weeks ago

After checking the code, it seems this bug might stem from commit c89c94c where the if statement to "cap" the severity was removed by @HHobeck.

If I downgrade GitVersion to the latest release prior to that commit (6.0.0-beta.1), the bumping works as expected.

HHobeck commented 2 weeks ago

I think we need to change the documentation.

Sharparam commented 2 weeks ago

As in: It bumping to 1.0.0 instead of 0.2.0 is intended? That goes against what semver.org recommends.

But in that case, can an option be added to get SemVer-compliant behaviour again?

asbjornu commented 2 weeks ago

@Sharparam where on SemVer.org is it recommended to interpret +major as +minor if the major version is 0? It does say that for major version 0 it's allowed to introduce breaking changes on minor bumps, so perhaps +semver:breaking in theory could increment minor while +semver:major would increment major? That causes +semver:breaking to have different effects depending on what the current version number is, which may be confusing?

Sharparam commented 2 weeks ago

I suppose it doesn't outright recommend either way. But given that the first 0 is static during the initial development, no change should be made to it.

And yeah, having "breaking" and "major" do different things would be confusing. But I don't see how it would make sense to have the tool inadvertently create a 1.0.0 version when you wouldn't expect it to (I certainly didn't when committing a breaking change during the 0.x phase).

The 1.0.0 version should be a very conscious decision (explicitly making the tag).

With the current behaviour, projects using GitVersion would always go directly from 0.0.x or 0.x.0 to 1.0.0, unless you explicitly don't use the commit message keywords and only use Git tags.

Or one can work around it by knowingly using the wrong bump syntax to get it to bump the correct version part (I had to write +semver:minor instead). But then the commit message doesn't make sense when you read it later.

HHobeck commented 2 weeks ago

May I ask you what the motivation is behind this behavior? What does it mean to be in an initial development phase? And if you are not already live with a version on production why do you care about breaking changes? Breaking a non-released feature makes no sense in my opinion. What information it gives to you when using breaking instead of minor keyword (especially in the initial development phase)?

Anyway it feels like +semver: breaking has a special semantic and needs to be (if we want to support this behavior) separated from the major-version-bump-message configuration.

Sharparam commented 2 weeks ago

Initial development being anything before the 1.0.0 release that can be considered feature-complete and stable. (At least that's how I view it.)

As a practical example, I have a Factorio mod that is in 0.x until a planned GUI feature for it has been added, that will be the first proper release with all expected features added. Since users are using this mod in their games, information about breaking changes is still very much useful. (In this case it's not using GitVersion though, but the same principles apply.)

As another example: A Factorio modding/API library I have that doesn't have all the features added yet for its actual usecase. But there are other parts of it that can still be used to interact with some APIs which people might find useful, and informing of breaking changes to the library is important.

Separating the major/minor/patch keywords from the more "subjective" ones like breaking sounds like it could be a good idea. major/minor/patch can just be hardcoded to always bump the relevant part of the version in that case, while breaking could have a config option for what to bump.

HHobeck commented 1 week ago

https://github.com/GitTools/GitVersion/pull/4185#issuecomment-2328252930

Then it might make more sense to adjust this new test to only consider +breaking, +feature and '+fix', if they're going to be treated separately?

Yes this might be on possibility to keep the logic like it is and create an alternative interpretation just for +breaking. Another possibility is to make it configurable (with an additional configuration parameter) and downgrade the major -> minor, minor -> patch and patch -> none when in the initial phase (0.x.x phase). But I'm honest: I have not really understood the use case.

@asbjornu, @arturcic : What do you think?

asbjornu commented 1 week ago

I think +semver:major should always increment major, regardless of what the current version number is. Whether +semver:breaking, semver:feature and semver:fix should be treated differently when the current version number is 0.x is something I'm fine with as long as it's properly tested and documented.

HHobeck commented 1 week ago

I think +semver:major should always increment major

The same holds true for minor and patch I assume?

asbjornu commented 1 week ago

@HHobeck yes.