Skip to content
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

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

Closed
2 tasks done
Sharparam opened this issue Aug 29, 2024 · 14 comments · Fixed by #4308
Closed
2 tasks done

Comments

@Sharparam
Copy link

Sharparam commented Aug 29, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have searched issues to ensure it has not already been reported

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 added a commit to Sharparam/GitVersion that referenced this issue Aug 29, 2024
@Sharparam
Copy link
Author

Sharparam commented Aug 29, 2024

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
Copy link
Contributor

HHobeck commented Aug 29, 2024

I think we need to change the documentation.

@Sharparam
Copy link
Author

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
Copy link
Member

@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
Copy link
Author

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
Copy link
Contributor

HHobeck commented Aug 30, 2024

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
Copy link
Author

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
Copy link
Contributor

HHobeck commented Sep 4, 2024

#4185 (comment)

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
Copy link
Member

asbjornu commented Sep 4, 2024

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
Copy link
Contributor

HHobeck commented Sep 4, 2024

I think +semver:major should always increment major

The same holds true for minor and patch I assume?

@asbjornu
Copy link
Member

asbjornu commented Sep 4, 2024

@HHobeck yes.

@arturcic
Copy link
Member

arturcic commented Nov 13, 2024

@HHobeck honestly I would not change the way the +semver:major, +semver:minor and +semver:patch work for 0.x versions specifically. I think we need to remove this exception from the docs, as the code is already behaving the same way for 0.x and other versions. I would keep it consistent. As for the breaking part, I would not introduce a specific case for 0.x. I think if the user wants to have the mentioned behavior during the initial development phase, he better updates the major-version-bump-message configuration and excludes the breaking from the regex and add it to the minor-version-bump-message , and when he's in the stable phase, he can change the configuration back to the original. So the only think we need to do is to update the docs and exclude the exception, as it's not the way the code works

@Sharparam
Copy link
Author

@arturcic This issue should not be closed as "completed" as the change went in a different direction from what was proposed (sadly).

Additionally, I would suggest adding a warning to the docs that bumping via "breaking" in a commit message during 0.x phase will in fact bump the major version, as this will not be expected by some people. And add a note on the proposed workaround of changing the *-bump-message configuration.

@arturcic arturcic reopened this Nov 13, 2024
@arturcic
Copy link
Member

@Sharparam to be honest I would rather move the suggestion to a discussion, or even convert this issue into a discussion as it's a special case. I'd keep the documentation as close as possible to the defaults and the special cases should go into discussions.

I would suggest adding a warning to the docs that bumping via "breaking" in a commit message during 0.x phase will in fact bump the major version, as this will not be expected by some people.

That's exactly why I'm suggesting to convert into a discussion, as the majority will expect to have same behavior in both cases, for those with a different expectation we have discussions. I think discussions serves better this type of "special" cases, agree?

@arturcic arturcic reopened this Nov 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants