feat: resolve increments from merged branches - #5140
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 18f6910930
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e34bfa475
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
4e34bfa to
f6b83bb
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f6b83bb5c1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
f6b83bb to
52598be
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 52598be177
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
52598be to
1c8f29f
Compare
1c8f29f to
3f8211c
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8211c580
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d8823790f7
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5cee297f10
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bdafb9537f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9be4f76314
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2c4146240d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6d79ccee85
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
You are very fast implementing things :) Thank you. It feels a little bit like prototyping. Anyway, I think the table is wrong and I was thinking to remove the complexity with the null value. I'm not sure if we really need the use case where the result will be Increment.None. From the coneptual point of view the following table would make more sense:
What do you think? |
|
I agree with the four-case table. The nullable value still has a useful role at the configuration boundary: an omitted branch value means "inherit". But it should be resolved before increment selection. I would model the selector only with the two effective booleans:
That makes This is cleaner than the current six-case implementation. I would rework the selector types and matrix tests around the resolved source value rather than add another special case. |
|
Hey @fzlzjerry, How is the rework going? Hope everything goes well. :) |
|
The rework is done in
I also updated the PR description to match the new table. Validation on this commit:
|
|
Great thank you. I'm goign to execute some test scenarios and do the code review. I will inform you about the outcome soon. |
|
@fzlzjerry make sure to rewrite the git history in an atomic and logically grouped way. Make it more fluent to read |
I think for now it is important to ge this working. At the end we can do one single commit with a nice commit message. @fzlzjerry: I'm cursios about IncrementStrategyFinder::HasCompatibleMainBranchHistory. Can you check the following scenarios please? (This is a basis for discussion) [TestCase(false, IncrementStrategy.Patch, false, "1.1.0-2")]
[TestCase(false, IncrementStrategy.Patch, true, "1.1.0-2")]
[TestCase(true, IncrementStrategy.Patch, false, "1.0.1-2")]
[TestCase(true, IncrementStrategy.Patch, true, "1.1.0-2")]
[TestCase(false, IncrementStrategy.Major, false, "2.0.0-2")]
[TestCase(false, IncrementStrategy.Major, true, "1.1.0-2")]
[TestCase(true, IncrementStrategy.Major, false, "2.0.0-2")]
[TestCase(true, IncrementStrategy.Major, true, "1.1.0-2")]
public void HonorsPreventIncrement1(
bool preventIncrementOfMergedBranch,
IncrementStrategy incrementStrategy,
bool preventIncrementWhenBranchMerged, string expected)
{
var configuration = GitFlowConfigurationBuilder.New
.WithBranch("main", builder => builder
.WithDeploymentMode(DeploymentMode.ContinuousDelivery)
.WithIncrement(IncrementStrategy.Minor)
.WithPreventIncrementOfMergedBranch(preventIncrementOfMergedBranch)
).WithBranch("hotfix", builder => builder
.WithIncrement(incrementStrategy)
.WithPreventIncrementWhenBranchMerged(preventIncrementWhenBranchMerged)
).Build();
using var fixture = new EmptyRepositoryFixture("main");
fixture.MakeATaggedCommit("1.0.0");
fixture.BranchTo("hotfix/foo");
fixture.MakeACommit();
fixture.MergeTo("main");
fixture.AssertFullSemver(expected, configuration);
}
[TestCase(false, IncrementStrategy.Patch, false, "1.1.0-2")]
[TestCase(false, IncrementStrategy.Patch, true, "1.1.0-2")]
[TestCase(true, IncrementStrategy.Patch, false, "1.0.1-2")]
[TestCase(true, IncrementStrategy.Patch, true, "1.1.0-2")]
[TestCase(false, IncrementStrategy.Major, false, "2.0.0-2")]
[TestCase(false, IncrementStrategy.Major, true, "1.1.0-2")]
[TestCase(true, IncrementStrategy.Major, false, "2.0.0-2")]
[TestCase(true, IncrementStrategy.Major, true, "1.1.0-2")]
public void HonorsPreventIncrement2(
bool preventIncrementOfMergedBranch,
IncrementStrategy incrementStrategy,
bool preventIncrementWhenBranchMerged, string expected)
{
var configuration = TrunkBasedConfigurationBuilder.New
.WithBranch("main", builder => builder
.WithDeploymentMode(DeploymentMode.ContinuousDelivery)
.WithIncrement(IncrementStrategy.Minor)
.WithPreventIncrementOfMergedBranch(preventIncrementOfMergedBranch)
).WithBranch("hotfix", builder => builder
.WithIncrement(incrementStrategy)
.WithPreventIncrementWhenBranchMerged(preventIncrementWhenBranchMerged)
).Build();
using var fixture = new EmptyRepositoryFixture("main");
fixture.MakeATaggedCommit("1.0.0");
fixture.BranchTo("hotfix/foo");
fixture.MakeACommit();
fixture.MergeTo("main");
fixture.AssertFullSemver(expected, configuration);
}
[TestCase(false, IncrementStrategy.Patch, false, "1.1.0-foo.2")]
[TestCase(false, IncrementStrategy.Patch, true, "1.1.0-foo.2")]
[TestCase(true, IncrementStrategy.Patch, false, "1.0.1-foo.2")]
[TestCase(true, IncrementStrategy.Patch, true, "1.1.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, false, "2.0.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, true, "1.1.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, false, "2.0.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, true, "1.1.0-foo.2")]
public void HonorsPreventIncrement3(
bool preventIncrementOfMergedBranch,
IncrementStrategy incrementStrategy,
bool preventIncrementWhenBranchMerged, string expected)
{
var configuration = GitFlowConfigurationBuilder.New
.WithBranch("feature", builder => builder
.WithDeploymentMode(DeploymentMode.ContinuousDelivery)
.WithIncrement(IncrementStrategy.Minor)
.WithPreventIncrementOfMergedBranch(preventIncrementOfMergedBranch)
).WithBranch("hotfix", builder => builder
.WithIncrement(incrementStrategy)
.WithPreventIncrementWhenBranchMerged(preventIncrementWhenBranchMerged)
).Build();
using var fixture = new EmptyRepositoryFixture("main");
fixture.MakeATaggedCommit("1.0.0");
fixture.BranchTo("feature/foo");
fixture.BranchTo("hotfix/next");
fixture.MakeACommit();
fixture.MergeTo("feature/foo");
fixture.AssertFullSemver(expected, configuration);
}
[TestCase(false, IncrementStrategy.Patch, false, "1.1.0-foo.2")]
[TestCase(false, IncrementStrategy.Patch, true, "1.1.0-foo.2")]
[TestCase(true, IncrementStrategy.Patch, false, "1.0.1-foo.2")]
[TestCase(true, IncrementStrategy.Patch, true, "1.1.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, false, "2.0.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, true, "1.1.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, false, "2.0.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, true, "1.1.0-foo.2")]
public void HonorsPreventIncrement4(
bool preventIncrementOfMergedBranch,
IncrementStrategy incrementStrategy,
bool preventIncrementWhenBranchMerged, string expected)
{
var configuration = TrunkBasedConfigurationBuilder.New
.WithBranch("feature", builder => builder
.WithDeploymentMode(DeploymentMode.ContinuousDelivery)
.WithIncrement(IncrementStrategy.Minor)
.WithPreventIncrementOfMergedBranch(preventIncrementOfMergedBranch)
).WithBranch("hotfix", builder => builder
.WithIncrement(incrementStrategy)
.WithPreventIncrementWhenBranchMerged(preventIncrementWhenBranchMerged)
).Build();
using var fixture = new EmptyRepositoryFixture("main");
fixture.MakeATaggedCommit("1.0.0");
fixture.BranchTo("feature/foo");
fixture.BranchTo("hotfix/next");
fixture.MakeACommit();
fixture.MergeTo("feature/foo");
fixture.AssertFullSemver(expected, configuration);
}
[TestCase(false, IncrementStrategy.Patch, false, "0.3.0-foo.2")]
[TestCase(false, IncrementStrategy.Patch, true, "0.3.0-foo.2")]
[TestCase(true, IncrementStrategy.Patch, false, "0.2.1-foo.2")]
[TestCase(true, IncrementStrategy.Patch, true, "0.3.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, false, "1.0.0-foo.2")]
[TestCase(false, IncrementStrategy.Major, true, "0.3.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, false, "1.0.0-foo.2")]
[TestCase(true, IncrementStrategy.Major, true, "0.3.0-foo.2")]
public void HonorsPreventIncrement5(
bool preventIncrementOfMergedBranch,
IncrementStrategy incrementStrategy,
bool preventIncrementWhenBranchMerged, string expected)
{
var configuration = GitFlowConfigurationBuilder.New
.WithBranch("feature", builder => builder
.WithDeploymentMode(DeploymentMode.ContinuousDelivery)
.WithIncrement(IncrementStrategy.Minor)
.WithPreventIncrementOfMergedBranch(preventIncrementOfMergedBranch)
).WithBranch("main", builder => builder
.WithIncrement(incrementStrategy)
.WithPreventIncrementWhenBranchMerged(preventIncrementWhenBranchMerged)
).Build();
using var fixture = new EmptyRepositoryFixture();
fixture.MakeACommit("A");
fixture.ApplyTag("0.1.0");
fixture.BranchTo("feature/foo");
fixture.MakeACommit("B");
fixture.Checkout("main");
fixture.MakeACommit("C");
fixture.ApplyTag("0.2.0");
fixture.MergeTo("feature/foo");
fixture.AssertFullSemver(expected, configuration);
} |
|
In MergeCommitOnNonTrunkBase and MergeCommitOnTrunkBase use the following method to determine the increment. We need to align the desired behavior for the sceanrio true / true on all workflows: context.Increment = ConsolidateIncrement(commit, context, baseVersion);
...and private static VersionField ConsolidateIncrement(
MainlineCommit commit, MainlineContext context, BaseVersion baseVersion)
{
var increment = VersionField.None;
var effectiveConfiguration1 = commit.GetEffectiveConfiguration(context.Configuration);
if (!effectiveConfiguration1.PreventIncrementOfMergedBranch)
{
increment = increment.Consolidate(context.Increment);
}
var effectiveConfiguration2 = commit.ChildIteration!.GetEffectiveConfiguration(context.Configuration);
if (!effectiveConfiguration2.PreventIncrementWhenBranchMerged)
{
increment = increment.Consolidate(baseVersion.Operator?.Increment);
}
if (effectiveConfiguration1.PreventIncrementOfMergedBranch &&
effectiveConfiguration2.PreventIncrementWhenBranchMerged)
{
increment = increment.Consolidate(context.Increment);
}
if (effectiveConfiguration1.CommitMessageIncrementing != CommitMessageIncrementMode.Disabled)
{
increment = increment.Consolidate(commit.Increment);
}
return increment;
} |
Description
Moves merged-branch increment selection into
IncrementStrategyFinder.DetermineIncrementedField, so every version strategy that already uses the finder receives the same behavior without adding a separate strategy or configuration surface.For a branch with merge-message tracking enabled that is either an effective main branch or a compatible descendant preserving main-branch history through linear work and main-to-descendant updates, the finder separates first-parent target work from recognized two-parent merges. It calculates each source branch's effective increment from the merged history, preserves source
+semver/=semverhandling, and applies the target/source settings as follows:of-merged-branchwhen-branch-mergedfalsefalsefalsetruetruefalsetruetrueMain-to-descendant update merges carry main's recursively derived version floor rather than treating main as a completed source branch. Unrelated intervening merges still keep the descendant outside merge-history processing.
Direct target commits still contribute the target increment, unrecognized merge histories remain target-configured work, synthetic pull-request refs stay outside merge-history processing, and the highest contribution across multiple merges wins. Inherited source increments are resolved from the historical merged tip, including deleted or recreated topic refs and source branches that absorb the topic later. Per-merge and ancestry results use configuration-aware cache keys because the finder evaluates multiple base-version candidates for the same current commit.
Related Issue
Resolves #4433
Motivation and Context
Outside mainline calculation, a no-fast-forward merge currently falls back to the target branch's configured increment. This makes a patch hotfix merged into a minor-increment main branch produce a minor version, and a minor feature merged into a patch-increment main branch produce a patch version. The increment finder already owns configured and commit-message increment precedence, so handling merged histories there keeps the behavior consistent across dependent version strategies.
How Has This Been Tested?
Tested on Linux with .NET SDK 10.0.400 at
b46b8201d:Inheritskipping.IncrementStrategyFinderunit case proving that commit-message cache entries are isolated by regex configuration.dotnet build ./src/GitVersion.slnx --no-restore --verbosity:minimal -m:1— succeeded with 0 warnings and 0 errors.dotnet test --solution ./src/GitVersion.slnx --no-build --no-restore --max-parallel-test-modules 1 --no-progress --output NormalwithTMPDIRandRUNNER_TEMPset to a clean non-repository directory — 37,290 succeeded, 0 failed.dotnet format --verify-no-changes ./src/GitVersion.slnx --no-restore— succeeded.git diff --check— succeeded.Screenshots (if appropriate):
N/A
Checklist: