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

Classic version and MajorMinorPatchMetadata #391

Closed
GeertvanHorrik opened this issue Mar 14, 2015 · 21 comments · Fixed by #427
Closed

Classic version and MajorMinorPatchMetadata #391

GeertvanHorrik opened this issue Mar 14, 2015 · 21 comments · Fixed by #427
Assignees
Milestone

Comments

@GeertvanHorrik
Copy link
Contributor

VariableProvider resets the CommitsSinceTag:

                // For continuous deployment the commits since tag gets promoted to the pre-release number
                semanticVersion.PreReleaseTag.Number = semanticVersion.BuildMetaData.CommitsSinceTag;
                semanticVersion.BuildMetaData.CommitsSinceTag = null;

So the AssemblySemVer will never have the right last number?

  1. Why is it reset to null? Am I not understand the VersioningMode correctly?
  2. A possible fix would be this change in AssemblyVersionsGenerator:
return string.Format("{0}.{1}.{2}.{3}", sv.Major, sv.Minor, sv.Patch, sv.BuildMetaData.CommitsSinceTag ?? sv.PreReleaseTag.Number ?? 0);
@JakeGinnivan
Copy link
Contributor

This is only in continuous delivery mode which overwrites patch with number commits, doesn't make sense to change 1.2.3-beta.4 to 1.2.4-beta.4, just make it 1.2.4-beta.

Sent from my Windows Phone


From: Geert van Horrikmailto:notifications@github.com
Sent: ý14/ý03/ý2015 15:33
To: ParticularLabs/GitVersionmailto:GitVersion@noreply.github.com
Subject: [GitVersion] Classic version and MajorMinorPatchMetadata (#391)

VariableProvider resets the CommitsSinceTag:

            // For continuous deployment the commits since tag gets promoted to the pre-release number
            semanticVersion.PreReleaseTag.Number = semanticVersion.BuildMetaData.CommitsSinceTag;
            semanticVersion.BuildMetaData.CommitsSinceTag = null;

So the AssemblySemVer will never have the right last number?

  1. Why is it reset to null? Am I not understand the VersioningMode correctly?
  2. A possible fix would be this change in AssemblyVersionsGenerator:

return string.Format("{0}.{1}.{2}.{3}", sv.Major, sv.Minor, sv.Patch, sv.PreReleaseTag.Number);


Reply to this email directly or view it on GitHubhttps://github.com//issues/391.

@GeertvanHorrik
Copy link
Contributor Author

But you set it to null for continuous deployment (so is this a bug?). I think there is a difference between 1.2.3-beta3 and 1.2.3-beta4.

@JakeGinnivan
Copy link
Contributor

It is inside if (mode == VersioningMode.ContinuousDeployment && !currentCommitIsTagged)

I actually meant continuous deployment in my example above.

The logic and reasoning is explained in the readme at https://github.com/ParticularLabs/GitVersion#octopus-deployci-build-nuget-packages

@GeertvanHorrik
Copy link
Contributor Author

Still not sure I understand, but let's close this issue.

@JakeGinnivan
Copy link
Contributor

In short, there are two modes. Continuous delivery and continuous deployment.

Continuous delivery will bump the BuildMetaData.CommitsSinceTag each commit which doesn't change the SemVer.

Continuous deployment mode promotes the changing CommitsSinceTag to be the Pre-release number. And I didn't see any point having the same number duplicated as you get

1.2.3-beta.1+1
1.2.3-beta.2+2
etc

I just drop the +1 by nulling it out. Make sense?

@JakeGinnivan
Copy link
Contributor

If I can improve the docs to explain better or something then I am interested in doing that, just not sure of the exact issue you are reporting so my answers might be missing

@GeertvanHorrik
Copy link
Contributor Author

So you are saying, in Continuous deployment, I should be able to get 1.2.3.1 when I specific MajorMinorPatchMetadata?

@JakeGinnivan
Copy link
Contributor

Hrrmm, ok I think the problem is the MajorMinorPatchMetadata itself.. we never take into account the build number.. So 1.2.3-beta.4+5 will either be 1.2.3.4 or 1.2.3.0, which is wrong as you say. I am following you now..

I think we should drop MajorMinorPatchMetadata, it doesn't make sense.. What do you think?

@JakeGinnivan
Copy link
Contributor

I think falling back through different things is not great.

Out of interest what are you using it for?

@JakeGinnivan
Copy link
Contributor

or we change it to be MajorMinorPatchTag so 1.2.3-beta.4+5 will be 1.2.3.4 or in continuous delivery mode it will be 1.2.3.5.

Then the issue is with stable, when it is stable we start bumping the patch on every commit meaning it will always be .0 at the end.

At any rate, I think this is a bad idea as we are trying to make a tool for SemVer?

@GeertvanHorrik
Copy link
Contributor Author

What I did in the past was using ClassicVersion, but that is dropped now in v3. I was looking for the alternative, so found AssemblySemVer (or something like that). However, I found it strange that 2.1.0-unstable.13 wouldn't result in version 2.1.0.13.

Then I created this issue. Then I decided: hmmm, 2.1.0 isn't a bad idea. I will keep 2.1.0 as file version, but use the FullSemVer as InformationalVersion, so decided to put back MajorMinorPatch.

So that's a bit of history. However, I think if a user wants to stamp it as 2.1.0.13, it should be allowed using MajorMinorPatchMetadata. If the user wants 2.1.0.0, he/she should use MajorMinorPatch (which is the default).

@JakeGinnivan
Copy link
Contributor

Makes sense. Sorry, missed the point of the original issue. Will have a think and add some tests around it to see what makes sense

@JakeGinnivan JakeGinnivan reopened this Mar 16, 2015
@JakeGinnivan JakeGinnivan self-assigned this Mar 17, 2015
@JakeGinnivan JakeGinnivan added this to the 3.0.0 milestone Mar 17, 2015
JakeGinnivan added a commit to JakeGinnivan/GitVersion that referenced this issue Apr 23, 2015
…mblyVersioningScheme.MajorMinorPatchTag

The issue with metadata is that it is not part of the semantic version at all, so why are we even allowing it to be put into the assembly version. On the other hand in continuous delivery we promote metadata to the tag to allow the version to change every commit. By using the tag, not the metadata we can create meaningful AssemblySemVer values.

Fixes GitTools#391
@GeertvanHorrik
Copy link
Contributor Author

Just running into this issue today when trying to release my 2nd beta version (2 commits). The NuGet version being generated is 2.2.0-beta0001 for both, so no update needed :-(

@GeertvanHorrik
Copy link
Contributor Author

What we can do:

  1. if you want to keep the beta1+1 (not sure why, but ok), we can use this numbering: beta10001, then beta2+1 becomes beta20001. Each next one becomes beta20002, etc.
  2. if you don't want to keep the beta1+1, we can just count the 0001, 0002, etc

@GeertvanHorrik
Copy link
Contributor Author

Let me know what you prefer and I will do a PR. Need to fix this today anyway to allow users to update to beta 2.

@MeirionHughes
Copy link
Contributor

"The NuGet version being generated is 2.2.0-beta0001" ...
that sounds a lot like the issue I had (meta count not equal to commit count).

[Edit] nvm; its probably not :P

@GeertvanHorrik
Copy link
Contributor Author

Just checked how something could become beta 2 (when applying an actual tag). Not sure if that's really required (anyone uses that?). To make sure I don't break anything, I will go for option 1.

@JakeGinnivan
Copy link
Contributor

Based on a discussion, I think the continuous deployment mode fixes this issue @GeertvanHorrik?

@GeertvanHorrik
Copy link
Contributor Author

Exactly. I have moved all my projects to continuous deployment and it works now, thanks.

@JakeGinnivan
Copy link
Contributor

If you can figure a better way to explain in readme a pr would be good

Sent from my Windows Phone


From: Geert van Horrikmailto:notifications@github.com
Sent: ‎28/‎05/‎2015 07:56
To: ParticularLabs/GitVersionmailto:GitVersion@noreply.github.com
Cc: Jake Ginnivanmailto:jake@ginnivan.net
Subject: Re: [GitVersion] Classic version and MajorMinorPatchMetadata (#391)

Closed #391#391.


Reply to this email directly or view it on GitHubhttps://github.com//issues/391#event-315690302.

@GeertvanHorrik
Copy link
Contributor Author

I think we should focus on #446 , that will make life much easier and more self explanatory.

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 a pull request may close this issue.

3 participants