-
Notifications
You must be signed in to change notification settings - Fork 654
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 for #1348, branch without commit take version from tag instead of branch name. #1743
Conversation
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.
Could you please add tests to make sure this change works as expected?
This issue has been automatically marked as stale because it has not had recent activity. After 30 days from now, it will be closed if no further activity occurs. Thank you for your contributions. |
I still have no good idea how to fix this. |
Any hint on what parts of the code that need modifications? I might have some time next week to look into this. |
I'm not sure, but you might want to take a look at Line 25 in 4510ab1
|
if (taggedSemanticVersion != null) | ||
{ | ||
// set the commit count on the tagged ver | ||
taggedSemanticVersion.BuildMetaData.CommitsSinceVersionSource = semver.BuildMetaData.CommitsSinceVersionSource; | ||
// replace calculated version with tagged version only if tagged version greater or equal to calculated version |
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.
Do you think we can use semver > taggedSemanticVersion instead? as the SemanticVersion implements IComparable
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.
Full semver compare leads to failure in some tests, eg. DevelopScenarios.CanHandleContinuousDelivery -> 1.1.0-alpha.8+1 instead of 1.1.0-alpha.7
At first sight it seems GitVersion is calculating wrong.
@pi3k14 I pushed some changes to you branch, can you rebase on top of master? |
@pi3k14 the build is failing, I'll need to fix it, as it's something with the scripts, when it gets fixed you can rebase once again and we can merge |
@arturcic it seems like the build scripts got confused by me naming the branch after the original issue |
It's something wrong only on Azure Pipelines, and other PRs have this issue. I'll try to find some time to fix the build |
@pi3k14 the build was fixed can you rebase on top of master and we can continue? |
@asbjornu do you mind having a look at this PR? |
Thank you so much for your contributions, @pi3k14 👍 |
You're welcome :) |
A start for resolving #1348.