Allow user control of build number integer

[AArnott]

This allows folks who want to do particularly custom things with their versioning stamps to have a bit more data from the version.json file with which to work.

Fixes #115

[AArnott]

@onovotny can you confirm this will unblock you?

[clairernovotny]

I think so...? I need to play with it.

I think what I need then is a custom targets to build the version from that, right?

[AArnott]

Yes. Do you mind playing with based on the package on AppVeyor's package feed (https://ci.appveyor.com/nuget/nerdbank-gitversioning) from this PR?

[clairernovotny]

Will try it out tomorrow or over the weekend and get back to you, thanks!

[clairernovotny]

Still looking at it, but one thing I noticed is that if I call Get-Version.ps1 without any version.json file, the BuildNumberFromVersionOptions results in -1. Not sure in that case if it should be 0 or match the regular BuildNumber or just be -1?

[clairernovotny]

I noticed that other than returning the value, everything else that uses it ignore it -- CloudBuildNumber, AssemblyInformationalVersion, etc.

Possibly, one suggestion is that if BuildNumberFromVersionOptions is > -1, then it uses that everywhere it currently uses the Height and then puts the Height in the metadata -- so: if I have 3.2.4-alpha in my version.json with a height of 5, here's what I'd expect:

CloudBuildNumber              : 3.2.4-alpha.5+gfa79893ea1
CloudBuildNumberEnabled       : True
BuildMetadataWithCommitId     : {gfa79893ea1}
AssemblyVersion               : 3.2.4.5
AssemblyFileVersion           : 3.2.4.5
AssemblyInformationalVersion  : 3.2.4-alpha.5+gfa79893ea1
PublicRelease                 : False
PrereleaseVersion             : -alpha
SimpleVersion                 : 3.2.4
BuildNumber                   : 5
BuildNumberFromVersionOptions : 4
MajorMinorVersion             : 3.2
GitCommitId                   : fa79893ea12f683b0e6ec2c31043e82cd0e22089
GitCommitIdShort              : fa79893ea1
VersionHeight                 : 5
Version                       : 3.2.4.5
CloudBuildVersionVarsEnabled  : True
CloudBuildVersionVars         : {[GitAssemblyInformationalVersion, 3.2.4-alpha.5+gfa79893ea1], [GitBuildVersion,
                                3.2.4.5]}
BuildMetadata                 : {}
BuildMetadataFragment         : +gfa79893ea1
NuGetPackageVersion           : 3.2.4-alpha-5-gfa79893ea1
NpmPackageVersion             : 3.2.4-alpha.5-gfa79893ea1
SemVer1                       : 3.2.4-alpha-5-gfa79893ea1
SemVer2                       : 3.2.4-alpha.5+gfa79893ea1

If I build on this PR to make those changes based on the value of BuildNumberFromVersionOptions, would you accept that PR? It would make using this much easier too as I'd only need a custom target to make the NuGet packages use the SemVer2 version instead of the NuGetPackageVersion.

[AArnott]

I had briefly considered actually consuming the number when we first chatted about the idea. There are actually several projects that do specify a 3rd integer right now (of zero) in their version that is currently ignored so NB.GV changing its behavior based on its presence would be a breaking change. That might be tolerable, but I'll have to buy into the idea first and that takes more time. :)

My first inclination is that if NB.GV was going to change its built-in logic to allow for this, I may perhaps let you describe it in the version.json with macros, for example:

{
   "version": "2.4.3-alpha.{height}"
}

Which you could then put height anywhere. I need to think about that more though.

In the meantime, yes I'm concerned about exposing this BuildNumberFromVersionOptions without any other changes because it looks wrong when you look at the rest of the properties I set. So that's mainly why I don't want to complete this PR unless (at least) it works for you. And perhaps I'll just throw it out in favor of a more general enhancement as described above.

[clairernovotny]

One possible approach is to control it by adding another property similar to how GitVersion does it -- an enum that has MajorMinor or MajorMinorPatch. The default (missing) value would be MajorMinor so it wouldn't break anyone today. Then, it could e set to MajorMinorPatch to use that third value everywhere instead.

This could be in addition to any templating improvements. I could imagine that people could find an AssemblyInformationlVersion template useful too, potentially with different values than the regular version.

[AArnott]

One thing I'd need to figure out first is where would I put the git height if MajorMinorPatch were specified.

1. I could force a 4th integer in the version. Inconsistent with semver but allowed basically everywhere. But folks who want to set MajorMinorPatch may not want git height to show up anywhere in the released version numbers.
2. I could put it in the +buildMetadata or -prerelease section, but if none is provided in the template/version field, would I just put the height as the very first identifier? e.g. 1.0.0-5? That looks a little wonky.

Another concern I have with this whole thing is that one tenet of NB.GV is that every commit produces a unique versioned package. But in the releasing branch (e.g. master) where a pre-release tag is not acceptable to include, a MajorMinorPatch setting would produce the same version of package for a series of commits. The best I could hope for is to include the height in +buildMetadata, but that does nothing for package version uniqueness, per semver rules.

[clairernovotny]

I think that having the height as a +buildMetadata for release versions is consistent with SemVer since it's arguable that the height is analogous to a build. I think I would be in the first camp where I want the height in the prerelease tags but not in any real version number (with the possible exception of AssemblyFileVersion as I think it's totally ok to have the fourth version be the buildnumber/height.

I think they key here is that MajorMinorPatch gives the best of both worlds -- guaranteed unique versions per commit (if you include the buildMetadata) but with more control over what is presented to the user in most cases.

[AArnott]

I think they key here is that MajorMinorPatch gives the best of both worlds

I'm mostly comfortable with users of NB.GV making that choice. I personally don't agree.

guaranteed unique versions per commit (if you include the buildMetadata)

Semver demands that +buildMetadata not be used to distinguish versions. So in fact you can't consider that as part of version uniqueness. So if you build to commits from master, they really will produce version-colliding packages.

That's why I'm so opposed to it.

with more control over what is presented to the user in most cases.

I just don't think it matters. I've never had a single user complain or express confusion at why a package jumped from 1.0.5 to 1.0.35.
How would you feel if you could control the first 3 numbers, but the 4th still existed? So it went from 1.0.1.5 to 1.0.2.35?

[clairernovotny]

I don't mind the fourth number existing, but I personally wouldn't use it for NuGet versions. I don't know of many packages that do a four part version. I know I could override that NuGet version in a Targets though.

In my primary workflow, I tend to rely on pushing to MyGet with a "preview" tag and then at some point, I'll push from MyGet to NuGet using MyGet's prerelease rewriting to remove the prerelease tag. I don't generally merge to Master and then do a build...no reason why I couldn't though and that might eventually be required if Package Signing ever becomes a thing making the rewriting impossible.

That said, MyGet does allow for replacing existing package versions so if there was a case where a package had the same NuGet version (but different internal 4 part versions stamped), MyGet would happily accept that. Clearly package caching is still an issue.

What do you think about AssemblyVersion? I know there's arguments on all sides due to binding redirects; some people advocate using only a major version for that, etc. Do you think it should be MajorMinor always, MajorMinorPatch or something else?

[AArnott]

I'll push from MyGet to NuGet using MyGet's prerelease rewriting to remove the prerelease tag

I personally avoid this feature for two reasons:

1. it leaves the -prerelease tag in the AssemblyInformationalVersion attribute of the assembly.
2. if I build that commit again, it will still include the prerelease tag.

So instead, when I'm ready to release a package, I strip the -prerelease from the version.json file and build again.

MyGet does allow for replacing existing package versions

Myget is only a small part of the problem. When I'm developing packages I often consume them locally as I'm developing them. If I can't commit and build again and get a unique version, that local package cache issue immediately becomes very troublesome.

What do you think about AssemblyVersion?

NB.GV already allows a lot of flexibility on this point. You can set AssemblyVersion to any major.minor version you want. Or you can set it to follow any of the 4 levels of precision in matching the AssemblyFileVersion. So you can do Major, Major.Minor, Major.Minor.Build, or Major.Minor.Build.Precision.

Personally, consistent with semver semantics, I prefer to lock AssemblyVersion to major.minor. Patches should be in-place swappable so a binding redirect change is unacceptable. So the 3rd integer must not be in the assembly version. But I am in favor of incrementing the minor component of an AssemblyVersion consistent with AssemblyFileVersion, and I bump the minor component whenever I add a feature, including adding any public APIs. This is because I find diagnosing an exception that states "I was looking for version 1.2.0.0 but found 1.1.0.0" much easier to diagnose and correct than a MissingMethodException is to figure out what in heck went wrong.

Binding redirects don't scare me. They are created automatically by the build anyway. So there's no big deal (modulo patch theory, as I've stated).

[clairernovotny]

One thing I just thought of where a three part version is mandatory is the appxmanifest of a store app. The fourth part of the version string there must be .0. Not sure if this warrants a StoreVersion calculation explicitly. For that purpose, I'd always be okay with MajorMinorHeight.0 regardless of what the version string is set to because it's not a value the user really sees.

[AArnott]

Interesting scenario. NB.GV doesn't have anything specific to support Store apps at this point. Where does the version that is set in the appxmanifest come from? Does the user have to bump it every time (in which case NB.GV is irrelevant) or can it be calculated by the build like source.extension.vsixmanifest files allow?

[clairernovotny]

The version is prompted in the wizard when done from the VS UI, but there's nothing special that does it from a CI build ... people have to write up a regex or XML-based task to set the version. Not too hard with PowerShell but still something that needs to be done. The version should be bumped for each release to the store/user machine.... basically the same as NuGet.

[AArnott]

Well, I'm not a fan of changing source code during the build. But if I could copy that file, change the version, and then point the build at the copied and modified file, that would work. But I'd wait for a customer to ask for it. I have no need for it myself as I don't develop store apps.

[clairernovotny]

Agreed -- not suggesting that NB.GV do it -- I guess the ask would just be to ensure that the MajorMinor / BuildNumber parts are all available independently so that someone's script could call Get-Version.ps1, get the object and then do what they need with the manifest.

[AArnott]

@onovotny If you don't see your scenario covered in the tests I added, or don't like the results they assert, please let me know. Actually, can you let me know either way? As I'd very much like your validation on this before I complete the PR.

[clairernovotny]

Will try it out with a few projects -- what's the best feed for this (myget, AppVeyor CI?) and are there any specifics I should look for in the MSBuild tasks and/or config as I play around with it?

[clairernovotny]

Found it...couple of thoughts:

1. Why is this version string valid? "2.3.0-beta.{height}" and this isn't? "2.3.0-beta-{height}"? When dealing with NuGet, it seems essential?
2. I'm okay with the restriction in 1, but still need a NuGet compatible version

Here is the build output I get with a version of "3.2.1-alpha.{height}" that has one commit past the branch and isn't on master:

CloudBuildNumber             : 3.2.1-alpha.1+gc8c11e0399
CloudBuildNumberEnabled      : True
BuildMetadataWithCommitId    : {gc8c11e0399}
AssemblyVersion              : 3.2.1.1
AssemblyFileVersion          : 3.2.1.1
AssemblyInformationalVersion : 3.2.1-alpha.1+gc8c11e0399
PublicRelease                : False
PrereleaseVersion            : -alpha.1
SimpleVersion                : 3.2.1
BuildNumber                  : 1
MajorMinorVersion            : 3.2
GitCommitId                  : c8c11e03999b82f160ed07d40d5648f5fd61491b
GitCommitIdShort             : c8c11e0399
VersionHeight                : 1
VersionHeightOffset          : 0
Version                      : 3.2.1.1
CloudBuildVersionVarsEnabled : True
CloudBuildVersionVars        : {[GitAssemblyInformationalVersion, 3.2.1-alpha.1+gc8c11e0399], [GitBuildVersion, 3.2.1.1]}
BuildMetadata                : {}
BuildMetadataFragment        : +gc8c11e0399
NuGetPackageVersion          : 3.2.1-alpha.1-gc8c11e0399
NpmPackageVersion            : 3.2.1-alpha.1-gc8c11e0399
SemVer1                      : 3.2.1-alpha.1-gc8c11e0399
SemVer2                      : 3.2.1-alpha.1+gc8c11e0399

Everything looks great, except for wanting the ability to not have the commit id in the prerelease tag for NuGet and/or SemVer1. It'd be nice if that was an option. In those case, it may be worth having an optional "padding" parameter where the height gets padded with 0's due to SemVer1/NuGet's sorting oddities.

As it is so far, none of the numbers are directly useful by NuGet.org since it doesn't allow SemVer 2; I'd need something like 3.2.1-alpha0001 or 3.2.1-alpha-0001 (I like the latter version better personally) where some configurable padding size was 4. To be clear, this special treatment could be a new output variable and doesn't have to replace any of the existing ones (maybe NuGetV2PackageVersion?) I really wish NuGet.org would fix this and just allow SemVer 2 versions to clients that support it.

With those, I think that should meet my needs. So far, it's looking really good though!

[AArnott]

Thanks for your review.

Why is this version string valid? "2.3.0-beta.{height}" and this isn't? "2.3.0-beta-{height}"? When dealing with NuGet, it seems essential?

Per (at least my reading of) semver rule 9, an identifier is only sorted numerically if it is entirely numeric. For instance, two identifiers "beta-5" and "beta-42" would sort incorrectly with 5 as superior to 42 I believe. But "beta.5" and "beta.42" would sort with 42 as superior to 5.
My understanding is nuget has (or is about to get) semver 2 support. Is your concern that nuget.org does not yet allow multiple identifiers in the prerelease tag? I already do one special handling of nuget package versions to deal with (at least previous) limitations of nuget. I may not be against another one so that versions can be proper semver2 whenever possible, and nuget compatible where necessary. Can you help me understand what limitation applies and how you recommend we workaround it?

it may be worth having an optional "padding" parameter where the height gets padded with 0's due to SemVer1/NuGet's sorting oddities.

That's an interesting one, if semver1 is actually interesting any more. But I'm curious whether it really is.

Everything looks great, except for wanting the ability to not have the commit id in the prerelease tag for NuGet and/or SemVer1. It'd be nice if that was an option.

It is an option. The prerelease identifier with the git commit ID is always added (whether building prerelease or not) if and only if you are not building a "public release". You can build with /p:publicrelease=true or (more typically) just add the publicReleaseRefSpec field to your version.json file to specify which branch (or tag) naming patterns should be interpreted as branch from which you publish releases (whether stable or unstable). Then NB.GV will automatically drop the commit ID identifier when building from those branches/tags.

I'll try to find some nuget.org docs on semver 2 support, unless you already know where they are.

[clairernovotny]

The format, 4.0.21-beta-23516, is definitely allowed on NuGet.org and required due to the sorting issues so far: https://www.nuget.org/packages/System.Runtime/4.0.21-beta-23516. It is not sorted numerically, that's the NuGet issue, it's sorted alphanumerically, which is why padding may be needed.

For NuGet SemVer v2 support, I've been hearing about it for awhile...and it'd solve so many hacks once they do.

Just yesterday, as I was publishing Rx.NET 4 preview 1, I had to go to annoying lengths since I couldn't publish 4.0.0-preview.1.

[AArnott]

I think I'll go with 4 digit padding, but make it configurable for folks who anticipate 5 digits.

[clairernovotny]

That sounds perfect, then the SemVer2 value can be used for the NuGet version at some point once nuget.org supports it (with much rejoicing).

[AArnott]

@onovotny Wanna take another look now?

what's the best feed for this (myget, AppVeyor CI?)

The appveyor feed: https://ci.appveyor.com/nuget/nerdbank-gitversioning

[clairernovotny]

Looking really good, I think I can use this in it's current form!

Here's with a non-public-release branch:

CloudBuildNumber                : 3.2.1-alpha.1+gc8c11e0399
CloudBuildNumberEnabled         : True
BuildMetadataWithCommitId       : {gc8c11e0399}
AssemblyVersion                 : 3.2.1.1
AssemblyFileVersion             : 3.2.1.1
AssemblyInformationalVersion    : 3.2.1-alpha.1+gc8c11e0399
PublicRelease                   : False
PrereleaseVersion               : -alpha.1
SimpleVersion                   : 3.2.1
BuildNumber                     : 1
MajorMinorVersion               : 3.2
GitCommitId                     : c8c11e03999b82f160ed07d40d5648f5fd61491b
GitCommitIdShort                : c8c11e0399
VersionHeight                   : 1
VersionHeightOffset             : 0
Version                         : 3.2.1.1
CloudBuildVersionVarsEnabled    : True
CloudBuildVersionVars           : {[GitAssemblyInformationalVersion, 3.2.1-alpha.1+gc8c11e0399], [GitBuildVersion, 3.2.1.1]}
BuildMetadata                   : {}
BuildMetadataFragment           : +gc8c11e0399
NuGetPackageVersion             : 3.2.1-alpha-0001-gc8c11e0399
NpmPackageVersion               : 3.2.1-alpha-0001-gc8c11e0399
SemVer1                         : 3.2.1-alpha-0001-gc8c11e0399
SemVer2                         : 3.2.1-alpha.1+gc8c11e0399
SemVer1NumericIdentifierPadding : 4

And here's the same as a public release

CloudBuildNumber                : 3.2.1-alpha.1
CloudBuildNumberEnabled         : True
BuildMetadataWithCommitId       : {gc8c11e0399}
AssemblyVersion                 : 3.2.1.1
AssemblyFileVersion             : 3.2.1.1
AssemblyInformationalVersion    : 3.2.1-alpha.1+gc8c11e0399
PublicRelease                   : True
PrereleaseVersion               : -alpha.1
SimpleVersion                   : 3.2.1
BuildNumber                     : 1
MajorMinorVersion               : 3.2
GitCommitId                     : c8c11e03999b82f160ed07d40d5648f5fd61491b
GitCommitIdShort                : c8c11e0399
VersionHeight                   : 1
VersionHeightOffset             : 0
Version                         : 3.2.1.1
CloudBuildVersionVarsEnabled    : True
CloudBuildVersionVars           : {[GitAssemblyInformationalVersion, 3.2.1-alpha.1+gc8c11e0399], [GitBuildVersion, 3.2.1.1]}
BuildMetadata                   : {}
BuildMetadataFragment           : +gc8c11e0399
NuGetPackageVersion             : 3.2.1-alpha-0001
NpmPackageVersion               : 3.2.1-alpha-0001
SemVer1                         : 3.2.1-alpha-0001
SemVer2                         : 3.2.1-alpha.1
SemVer1NumericIdentifierPadding : 4

[clairernovotny]

Actually, one catch -- AssemblyVersion. I commited and checked again:

CloudBuildNumber                : 3.2.1-alpha.2+g71b583a455
CloudBuildNumberEnabled         : True
BuildMetadataWithCommitId       : {g71b583a455}
AssemblyVersion                 : 3.2.1.2
AssemblyFileVersion             : 3.2.1.2
AssemblyInformationalVersion    : 3.2.1-alpha.2+g71b583a455
PublicRelease                   : False
PrereleaseVersion               : -alpha.2
SimpleVersion                   : 3.2.1
BuildNumber                     : 1
MajorMinorVersion               : 3.2
GitCommitId                     : 71b583a4556384cb9fae0b2c23e6bed1f4089d02
GitCommitIdShort                : 71b583a455
VersionHeight                   : 2
VersionHeightOffset             : 0
Version                         : 3.2.1.2
CloudBuildVersionVarsEnabled    : True
CloudBuildVersionVars           : {[GitAssemblyInformationalVersion, 3.2.1-alpha.2+g71b583a455], [GitBuildVersion, 3.2.1.2]}
BuildMetadata                   : {}
BuildMetadataFragment           : +g71b583a455
NuGetPackageVersion             : 3.2.1-alpha-0002-g71b583a455
NpmPackageVersion               : 3.2.1-alpha-0002-g71b583a455
SemVer1                         : 3.2.1-alpha-0002-g71b583a455
SemVer2                         : 3.2.1-alpha.2+g71b583a455
SemVer1NumericIdentifierPadding : 4

I should think the AssemblyVersion should be different than the height given binding redirect hell, no? Totally ok with the file version being incremented.

I can modify the values in targets, but just wondering about your thoughts around defaults?

[emgarten]

OK. I'll adjust this to automatically convert . to - and pad numeric identifiers for the NuGetPackageVersion and SemVer1 properties. That's why they're there.

That is the best way for now.

Combining - and . in semver2 is fine, but you only get sort of individual labels with a . between them.

[clairernovotny]

@emgarten any ETA on when NuGet.org will support SemVer2 packages? Would love to get rid of the hoop-jumping we're doing for v1 support.

[emgarten]

@onovotny not sure on the ETA but it is in progress. NuGet/NuGetGallery#3547

[AArnott]

I should think the AssemblyVersion should be different than the height given binding redirect hell, no? Totally ok with the file version being incremented.
I can modify the values in targets, but just wondering about your thoughts around defaults?

I agree. And NB.GV already supports and in facts defaults to something different than your output shows. For example, consider the output of the GetBuildVersion_WithThreeVersionIntegers test is:

AssemblyCompany = TestCompany
AssemblyConfiguration = Debug
AssemblyCopyright = TestCopyright
AssemblyFileVersion = 7.8.9.1
AssemblyInformationalVersion = 7.8.9-beta.3+g65c66203fb
AssemblyName = TestAssembly
AssemblyProduct = TestProduct
AssemblyTitle = TestAssembly
AssemblyVersion = 7.8.0.0
BuildNumber = 9
BuildVersion = 7.8.9.1
BuildVersion3Components = 7.8.9
BuildVersionNumberComponent = 9
BuildVersionSimple = 7.8.9
CloudBuildNumber = 
GitAssemblyInformationalVersion = 
GitBuildVersion = 
GitCommitId = 65c66203fbd21f972691015b32cc9bcd8fafd5de
GitCommitIdShort = 65c66203fb
GitVersionHeight = 1
LoggedEvents = System.Collections.Generic.List`1[Microsoft.Build.Framework.BuildEventArgs]
MajorMinorVersion = 7.8
NuGetPackageVersion = 7.8.9-beta-3-g65c66203fb
PrereleaseVersion = -beta.3
PublicRelease = False
RootNamespace = TestNamespace
SemVerBuildSuffix = +g65c66203fb

Notice how the assembly version is only the major.minor of the assembly file version. This is the default behavior. For the output you're getting, I expect you're changing the default using a version.json file such as this:

{
  "version": "3.2.1-alpha.{height}",
  "assemblyVersion": {
    "precision": "revision"
  }
}

But if you drop the assemblyVersion field you'd drop back to the default. Can you share your version.json file?

[clairernovotny]

I've pushed a branch here: https://github.com/Reactive-Extensions/Rx.NET/tree/nbgv, the file is here: https://github.com/Reactive-Extensions/Rx.NET/blob/nbgv/Ix.NET/Source/version.json

I do have

"assemblyVersion": {
    "precision": "revision"
  }

What should it be to get major/minor or major/minor/patch only?

[clairernovotny]

Answered my own question, looks like I need to use either minor or build here, https://github.com/AArnott/Nerdbank.GitVersioning/blob/4d80666c500dcab6ea6b6daaef2e47329dde9d90/src/NerdBank.GitVersioning/version.schema.json#L23

IMO :)

[AArnott]

OK, good. So you're seeing exactly what I expected you to see. Why are you setting precision to revision and then surprised that that's what you're seeing? (that's a question meant to assess usability and discoverability of my schema and docs).

Just drop the field entirely if you want the default major.minor behavior. There is a JSON schema file, but you're not getting it because your schema line is wrong. If you remove it, some editors will automatically pick up the right one since I have version.json registered with the JSON schema registry. But if it doesn't come up automatically, just set that line to:

 "$schema": "https://raw.githubusercontent.com/AArnott/Nerdbank.GitVersioning/master/src/NerdBank.GitVersioning/version.schema.json"

Then Intellisense in your editor should help you discover that precision is an enum that accepts "major", "minor", "build", "revision".

[clairernovotny]

I think I had it set from when I was trying to get it to work before this PR was redone :)

What do you generally think for assembly version, minor or build? I want to cause the least amount of pain for people while still being "correct".

[clairernovotny]

The one thing I'd suggest is to have a work item based on NuGet/NuGetGallery#3547 to flip the meaning of NuGetPackageVersion to SemVer2. Either that or have some easy way to update it using an MSBuild prop. Goal is to make it easy to switch once NuGet supports it.

[AArnott]

What do you generally think for assembly version, minor or build?

CoreFx folks prefer revision (assembly version === assembly file version). I disagree on this policy, but I don't fully understand why they have it.
I prefer minor. My interpretation of semver rules suggests that any new API in a library package warrants a minor version bump. Any change smaller than that (i.e. a bug fix) would just be a patch bump. I don't think bug fixes should require updating binding redirects. But I do think that any new API calls for callers to be aware (or redirect to) what they are using. That way, if you compile against 2.3 but somehow run against 2.2, you'll see a nice friendly exception about those two version numbers instead of a "Method X is not found" and leaving you to dig up why something broke.

[AArnott]

I'm stewing on the decision to bump this to version 2.0 and whether any more docs in the repo need to be updated to accommodate this change before merging. I hope to merge later tonight though.

One regression I anticipate from this change is that the Get-CommitId.ps1 script will fail to find commits when you specify more integers than just major.minor in the version field. I can live with that in the short term, I think. But if I can find a cheap way to fix it now, I'd prefer that.

[clairernovotny]

@AArnott one last Q, I think -- when it's time to do an actual release, what's the right way to do that? do I check in a new version.json without the prerelease tag or can I just tag a version and will it detect the tag and drop the pre-release label for that commit?

[AArnott]

when it's time to do an actual release, what's the right way to do that? do I check in a new version.json without the prerelease tag

Yes, but I personally usually don't wait for the actual release to do that. When the product is stable, I remove the unstable tag. I may not push a build to nuget.org for 20-30 more commits, but from the stable point on, my CI is building without the prerelease tag. And as I generally try to accompany any code changes with test additions, my code tends to always be stable (modulo a bug or two at times), so I usually use the prerelease tag for long-running feature development or leading up to v1 releases.

I also update the version.json as soon as I know I'm working on v.next where the version would be bumped. For example, if I shipped 1.1 and know that 1.2 is coming up next, I'll go ahead and bump version.json as soon as 1.1 ships rather than building 1.1.x packages until I'm about to ship 1.2 and update it at the last minute.

can I just tag a version and will it detect the tag and drop the pre-release label for that commit?

No. Tags can be added to commits later, so the presence of any tag makes no difference to the build or else it violates build reproducibility, which is a firm tenet with NB.GV. The only effect tags or branches can have on versioning is automatically applying /p:PublicRelease=true behavior when they match a regex in publicReleaseRefSpec, and you have to actually be building the commit in the context of that branch or tag (not merely have a tag on the commit being built) in order to get that behavior.

[clairernovotny]

@AArnott if that's not already in the docs, that process would make a great addition :)

[AArnott]

if that's not already in the docs, that process would make a great addition :)

The Overview section of the README covers the fact that branches and tags don't change the built version. But it doesn't go into the workflow of updating the version right after a release. Is that what you found interesting but not documented? I just want to double check what you thought should be documented before I take time to write it up.

[clairernovotny]

@AArnott Yep, I think the actual process of updating the version.txt/json around a release would be useful to spell out.

[clairernovotny]

Found one potential issue with the non public releases. Take the version 3.2.0-preview-0004-g183fe87eab as might be calculated for a "non public" branch that we might want to push to MyGet -- I believe that prerelease tags in NuGet are capped at 20 characters?

Seems like we'd need to mark that branch as a release branch -- wonder if it's worth any sort of warning text, but you have no way of really knowing if they're using the version for NuGet or something else.

[clairernovotny]

@AArnott FYI dotnet/reactive#402. Will switch to a "public" build once it's on NuGet.