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

Fix issue with empty prerelease tags. #3224

Conversation

enriqueraso
Copy link
Contributor

@enriqueraso enriqueraso commented Oct 6, 2022

Fix issue when you have an empty tag configured in your branch defined as mainline.

Description

Basically, the changes are related to detect if branchConfig is Mainline and has empty prerelease tag configured on it, then it will update prerelease tag in semver.

I created the branch from support/5.x

Related Issue

Fix #3218

Motivation and Context

I want to use GitVersion in Azure DevOps with the gitversion.yml described in the bug.

Also, gitversion.yml of the documentation has configured empty prerelease tag and mainline for main and support/* branches.

How Has This Been Tested?

I tested locally and from the Azure DevOps.

image

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@asbjornu
Copy link
Member

asbjornu commented Oct 11, 2022

Somehow this feels related to #3208 and #3223.

@enriqueraso
Copy link
Contributor Author

enriqueraso commented Oct 13, 2022

@asbjornu I updated this PR to reference active bug #3218. Wrongly I put closed bug.

Also, I reviewed #3218 and detailed a little bit more.

@enriqueraso
Copy link
Contributor Author

@asbjornu I checked my bug #3218 with proposal PR #3208 applying the changes in Config.cs and SemanticVersion.cs, and it doesn't work. GitVersion continues returning 1.14.0-rc on releases/1 branch although I have an empty string tag in configuration for releases branch.

PR #3208 is related to tag-prefix in configuration and I am fixing tag in branch configuration.

The image below shows result from GitVersion with changes from PR #3208 and from GitVersion with my PR. So, my PR solved another issue reported in bugs #3223 and #3060

image

@@ -116,6 +126,12 @@ private void UpdatePreReleaseTag(EffectiveConfiguration configuration, SemanticV
{
var tagToUse = configuration.GetBranchSpecificTag(this.log, Context.CurrentBranch.Name.Friendly, branchNameOverride);

if (configuration.IsMainline && tagToUse.IsEmpty())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why only in that case the branch is set to IsMainline=true?

@@ -80,8 +80,12 @@ public NextVersion FindVersion()
var hasPreReleaseTag = semver.PreReleaseTag?.HasTag() == true;
var tag = configuration.Value.Tag;
var branchConfigHasPreReleaseTagConfigured = !tag.IsNullOrEmpty();
var preReleaseTagDoesNotMatchConfiguration = hasPreReleaseTag && branchConfigHasPreReleaseTagConfigured && semver.PreReleaseTag?.Name != tag;
if (semver.PreReleaseTag?.HasTag() != true && branchConfigHasPreReleaseTagConfigured || preReleaseTagDoesNotMatchConfiguration)
var branchConfigIsMainlineAndHasEmptyPreReleaseTagConfigured = configuration.Value.IsMainline && tag.IsEmpty();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm surprised that this logic is so complex why not keep it simple and refactor the expression like:

var expectedTag = configuration.GetBranchSpecificTag(this.log, Context.CurrentBranch.Name.Friendly, branchNameOverride);
var actualTag = semver.PreReleaseTag?.Name ?? string.Empty;
var preReleaseTagDoesNotMatchConfiguration = actualTag != expectedTag;
if(preReleaseTagDoesNotMatchConfiguration)
{
    UpdatePreReleaseTag(configuration.Value, semver, expectedTag);
}

@asbjornu asbjornu force-pushed the bug/resolving-semver-fail-with-an-empty-tag-in-a-configuration-branch branch from ae8500f to 3a6b123 Compare January 20, 2023 10:55
@asbjornu asbjornu merged commit 3f75764 into GitTools:support/5.x Jan 20, 2023
@mergify
Copy link
Contributor

mergify bot commented Jan 20, 2023

Thank you @enriqueraso for your contribution!

arturcic pushed a commit that referenced this pull request Jan 20, 2023
Merge pull request #3224 from enriqueraso/bug/resolving-semver-fail-with-an-empty-tag-in-a-configuration-branch

Fix issue with empty prerelease tags.
arturcic pushed a commit that referenced this pull request Feb 16, 2023
Merge pull request #3224 from enriqueraso/bug/resolving-semver-fail-with-an-empty-tag-in-a-configuration-branch

Fix issue with empty prerelease tags.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Resolving SemVer fail with an empty tag in a configuration branch
3 participants