-
Notifications
You must be signed in to change notification settings - Fork 656
Extend git flow complex example to post merge and rebase behaviour #4585
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
base: main
Are you sure you want to change the base?
Extend git flow complex example to post merge and rebase behaviour #4585
Conversation
1586c8e
to
45f96cc
Compare
@HHobeck, I think these changes look reasonable. What are your thoughts on this? |
@@ -7,7 +7,7 @@ namespace GitVersion.Core.Tests.Helpers; | |||
|
|||
public class TestBase | |||
{ | |||
public const string MainBranch = "main"; | |||
public const string MainBranch = RepositoryFixtureBase.MainBranch; |
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.
#4589 Magic strings >> BranchConfigurationKey.
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.
May I ask you: Why do you care about how the branch is named in the unit tests? Does it make any different if it is named master or main? From the configuration point of view both are included and treated the same. At the end everyone can use their own name and their own configuration in the unit tests. There is no need for unification IMO.
fixture.MakeACommit( | ||
"feature 2 additional commit after original feature has been merged to develop " + System.Environment.NewLine + | ||
$"and release/1.2.0 has already happened >> {fullSemver}" + | ||
"Problem #1: 1.3.0-f2.1+0 is what I observe when I run dotnet-gitversion 6.3.0 but in the repo the assertion is 1.3.0-f2.1+1" + |
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.
Problem 1:
- 1.3.0-f2.1+1 is what the test asserts
- 1.3.0-f2.1+0 is what I observe when I run dotnet-gitversion 6.3.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.
$"and release/1.2.0 has already happened >> {fullSemver}" + | ||
"Problem #1: 1.3.0-f2.1+0 is what I observe when I run dotnet-gitversion 6.3.0 but in the repo the assertion is 1.3.0-f2.1+1" + | ||
"After rebase 1.3.0-f2.1+3 is both what the test asserts and what I observe when I run dotnet-gitversion 6.3.0." + | ||
"Problem #2: I expected to get the same before and after the rebase." + |
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.
Problem 2:
- Before rebase 1.3.0-f2.1+1 is what the test asserts (but not what dotnet-gitversion produces - see Problem 1).
- After rebase 1.3.0-f2.1+3 is what both tests and dotnet-gitversion 6.3.0 produce.
I'd have anticipated1.3.0-f2.1+1 both before and after rebase, in this specific scenario.
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.
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.
Odd. If I run the test now and flip out to the command line the config that's being persisted by the test is no longer valid for running with 6.3.0 but it works for you? I'll look again when it's a more sensible o'clock.
In the meantime, as I said on my opening post, if my expectations aren't correct then I would like to understand what would be correct.
1 - The merge bumps the root version on the same f2 commit from 1.2 to 1.3; that's an expected change due to changed state, and the +0 resets since there have now been no commits since version source. All fine.
2 - I'd expected the first commit on the branch since the last version bump to be f2.1+1 as the test asserts before rebase. I concur with the test. Dotnet-gitversion 6.3.0 tbc.
3 - If I rebase that commit, yes the whole tree is different but that first commit is no longer considered the first commit on the branch since the version source? I'd not expected that.
How does +1 jump to +3? If there's documentation, point me at it as I do appreciate you've been generous with your time already.
To be fair working out how to configure the majority minor patch bumps work now is what I'd expected to be playing with in the tests, not the CommitsSinceVersionSource idiosyncrasies...
var yaml = new ConfigurationSerializer().Serialize(concreteConfig); | ||
const string fileName = "GitVersion.yml"; | ||
var filePath = FileSystemHelper.Path.Combine(r.Info.Path, "..", fileName); | ||
File.WriteAllText(filePath, yaml); |
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.
Intentionally writing out the GitVersion.yml to demonstrate inconsistency between UnitTest version calculation and running dotnet-gitversion from the commandline. Refer to 2 problems in commentary.
45f96cc
to
8e1ddbd
Compare
Description
Extend the GitFlowComplexExample to document unexpected post merge and rebase behaviour.
Fix the integration test fixtures so they can accept a "master" main branch instead of hardcoding "main".
Related Issue
Motivation and Context
I have been really struggling to get GitVersion to work post the v6 changes so I checked out the repo to run through sample scenarios to try and adapt but I either need more documentation to understand what the new behaviour is or the new behaviour is possibly unintended.
Rather than just rely on the Integration tests with injected configuration, I wanted to be able to revisit the repository that is created by the integration test and I observe
How Has This Been Tested?
GitFlowComplexExample
Screenshots (if appropriate):
Checklist:
This is a WIP proposal to document a problem & ask for help; if I get direction I'm happy to update the PR as offered/advised.