GitVersion icon indicating copy to clipboard operation
GitVersion copied to clipboard

[Bug]? no minor version detected on pull request that contains organization name

Open ooredm opened this issue 3 years ago • 3 comments

Context: MainlineMode.

No minor increment on merge message that contains an organization name, e.g: "Merge pull request #1 from bananaCorp/develop" This is the default pull request message of GitHub for an organization.

The default regex for a develop branch is, "^dev(elop)?(ment)?$", due to this the branch is not detected by the MainLineVersionCalculator, as it sees "bananaCorp/develop" as the branch. We can work around this by changing the regex to "dev(elop)?(ment)?$".

My question is if this is the intended behavior, and for what reason.

Thanks in advance.

ooredm avatar Aug 31 '22 14:08 ooredm

Merge pull request https://github.com/GitTools/GitVersion/issues/1 from bananaCorp/develop

To me, this looks like a merge from a remote into a local develop branch, instead of what it imho should have been: A fast-forward or a rebase. Merges shouldn't be necessary between branches of the same name, regardless of whether they are remote or local.

Also, the fact that you're using a develop branch tells me that you aren't really doing Mainline development. In mode: Mainline, there is only one branch that matters: The main branch. The point is to use trunk based development where no branches are visible anywhere because they are squashed into single, no-merge commits on main. If you want to use more than one branch, you should not use mode: Mainline.

The default regex for a develop branch is, "^dev(elop)?(ment)?$", due to this the branch is not detected by the MainLineVersionCalculator, as it sees "bananaCorp/develop" as the branch. We can work around this by changing the regex to "dev(elop)?(ment)?$".

If you absolutely want to do remote merges the way you do and have branches named bananaCorp/develop, you can of course remove the start of line anchor ^ from the regex. 🤷🏼 The reason the regex only matches dev, develop and development by default is to ensure consistency through enforcing a Git Flow compliant development model.

asbjornu avatar Sep 01 '22 14:09 asbjornu

To me, this looks like a merge from a remote into a local develop branch, instead of what it imho should have been: A fast-forward or a rebase. Merges shouldn't be necessary between branches of the same name, regardless of whether they are remote or local.

It is actually a merge which is executed by Github when merging a pullrequest, creating a merge commit. I don't know why the organization name is added in the commit message, however the branch is plainly develop. So the merge is not performed locally from a remote. In this case the merge goes from develop to master, however Github leaves out the target branch in the merge message.

If you want to use more than one branch, you should not use mode: Mainline.

What mode would you suggest in this case?

koosvanw avatar Sep 02 '22 07:09 koosvanw

If you are closer to a Git Flow development model with develop, feature, release branches, and so on, using the default mode ContinuousDeployment or ContinuousDelivery would be better.

asbjornu avatar Sep 02 '22 08:09 asbjornu

I have taken a look into the code and saw unit tests which are testing the behavior as following:

    private static readonly object?[] GitHubPullPullMergeMessages =
    {
        new object?[] { "Merge pull request #1234 from feature/one", "feature/one", null, null, 1234 },
        new object?[] { "Merge pull request #1234 in feature/one", "feature/one", null, null, 1234  },
        new object?[] { "Merge pull request #1234 in v4.0.0", "v4.0.0", null, new SemanticVersion(4), 1234  },
        new object?[] { "Merge pull request #1234 from origin/feature/one", "origin/feature/one", null, null, 1234  },
        new object?[] { "Merge pull request #1234 in feature/4.1.0/one", "feature/4.1.0/one", null, new SemanticVersion(4,1), 1234  },
        new object?[] { "Merge pull request #1234 in V://10.10.10.10", "V://10.10.10.10", null, null, 1234 },
        new object?[] { "Merge pull request #1234 from feature/one into dev", "feature/one", "dev", null, 1234  }
    };

    [TestCaseSource(nameof(GitHubPullPullMergeMessages))]
    public void ParsesGitHubPullMergeMessage(
        string message,
        string expectedMergedBranch,
        string expectedTargetBranch,
        SemanticVersion expectedVersion,
        int? expectedPullRequestNumber)
    {
        // Act
        var sut = new MergeMessage(message, this.configuration);

        // Assert
        sut.FormatName.ShouldBe("GitHubPull");
        sut.TargetBranch.ShouldBe(expectedTargetBranch);
        sut.MergedBranch.ShouldBe(expectedMergedBranch);
        sut.IsMergedPullRequest.ShouldBeTrue();
        sut.PullRequestNumber.ShouldBe(expectedPullRequestNumber);
        sut.Version.ShouldBe(expectedVersion);
    }

What me supprised is that indeed like @ooredm explained it the organization name is (always?) part of the merge message in git hub: image

HHobeck avatar Mar 05 '23 13:03 HHobeck

@ooredm To workaround this I would suggest you to change the configuration and place the following string:

merge-message-formats:
  Custom: '^Merge pull request #(?<PullRequestNumber>\d+) (from|in) (?:[^\s\/]+\/)?(?<SourceBranch>[^\s]*)(?: into (?<TargetBranch>[^\s]*))*'

HHobeck avatar Mar 05 '23 13:03 HHobeck

Closed by #3424

arturcic avatar Mar 06 '23 12:03 arturcic

:tada: This issue has been resolved in version 6.0.0-beta.2 :tada: The release is available on:

Your GitReleaseManager bot :package::rocket:

arturcic avatar Apr 06 '23 19:04 arturcic