Adopt MinVer

[slang25]

This PR swaps out our (IMO good) versioning in favour of MinVer.
After reading the How it works section, I'm convinced it's nailed it, and does everything we currently want in terms of versioning (even thex.0.0.0 assembly version number).

I've used the MinVerMajorMinor property to mimic how we currently control the version number, but we could be worth dropping this and letting it discover it automatically. It would mean after releasing a v7.0.0, we'd create a v8.0.0-alpha.0 tag, then any PR builds would follow on from that (if that makes sense). This would help us avoid situations where a release is made without a tag, as that would be the only way to manage the version number.

[slang25]

We should also re-evaluate this script

[AnthonySteele]

You say "re-evaluate" I say "delete" with "fire".

[slang25]

Exactly!

[slang25]

We wouldn't need to do this is we opt to rely on tags only. If we did that then there's no risk of them not being aligned.

[adamralph]

This property doesn't affect the release process. The release is purely driven by the tag you create. Updating this property is actually something you'd do after releasing, in the case where you want to start work on a new major-minor range.

[slang25]

Thanks, I'll update this 👍

[martincostello]

Have you got this configured correctly? I just tried adding MinVer to a package of my own and it was generating "too many numbers" and the warning caused my build to fail.

Looking in the AppVeyor logs the same is happening here, but it still passes because warnings aren't errors in JustSaying:

C:\projects\justsaying\.dotnetcli\sdk\2.1.500\Sdks\NuGet.Build.Tasks.Pack\buildCrossTargeting\NuGet.Build.Tasks.Pack.targets(202,5): error NU5105: The package version '7.0.0-alpha.0.49+build.455' uses SemVer 2.0.0 or components of SemVer 1.0.0 that are not supported on legacy clients. Change the package version to a SemVer 1.0.0 string. If the version contains a release label it must start with a letter. This message can be ignored if the package is not intended for older clients. 

Specifically I think it's the alpha.0.49 part that's wrong, because it should be alpha.49?

[slang25]

I'll take a look this evening

[slang25]

Looks like this is relevant adamralph/minver#92

[martincostello]

That looks pertinent to the warning, but not the extra zero in the part before the +.

[slang25]

We can change this behaviour, but it is expected. The alpha.0 is the controlled part that will be inferred from tags. Do you think we should drop the number part?

Update: I see that my original comment may have caused this confusion.

[slang25]

Lets see what the outcome of this is: adamralph/minver#169

[adamralph]

Have you got this configured correctly? I just tried adding MinVer to a package of my own and it was generating "too many numbers" and the warning caused my build to fail.

Looking in the AppVeyor logs the same is happening here, but it still passes because warnings aren't errors in JustSaying:

C:\projects\justsaying\.dotnetcli\sdk\2.1.500\Sdks\NuGet.Build.Tasks.Pack\buildCrossTargeting\NuGet.Build.Tasks.Pack.targets(202,5): error NU5105: The package version '7.0.0-alpha.0.49+build.455' uses SemVer 2.0.0 or components of SemVer 1.0.0 that are not supported on legacy clients. Change the package version to a SemVer 1.0.0 string. If the version contains a release label it must start with a letter. This message can be ignored if the package is not intended for older clients. 

@martincostello You will get NU1505 when using MinVer, because it will produce SemVer 2.0 version numbers. The reason the build is failing is that you have configured your project to treat warnings as errors. As the warning says, you can safely ignore it if the package is not intended for older clients. "Older clients" in this case, means anything older than around September 2017. You can get rid of the warning like so: https://github.com/adamralph/bullseye/blob/794fbaee349ab4478921f4d5d51b2faaf28158fc/Bullseye/Bullseye.csproj#L8. If you do care about those older (ancient?) clients, then it's probably best not to use MinVer.

Specifically I think it's the alpha.0.49 part that's wrong, because it should be alpha.49?

@martincostello the version is alpha.0.49 because the current commit is 49 commits ahead of the last tag or root commit. The 49 is the height. This is explained in https://github.com/adamralph/minver#how-it-works

Lets see what the outcome of this is: adamralph/minver#169

@slang25 that PR will not remove NU1505. Even if you change the default pre-release to be someting like alpha0 (without the dot), you will still get NU1505 when you build any commit which has non-zero height above the root commit or the last tag, because MinVer will append .{height} to the pre-release. Note that the build metadata (+build.455) will also trigger NU1505, because build metadata was introduced in SemVer 2.0.

The reason the default pre-release identifiers are alpha.0 is to guarantee that they are lower than the first tag you will create, which at it the lowest, will be alpha.1. In that way, your commits will have the following versions:

• 7.0.0-alpha.0.49 (49 commits above the last tag or root commit)
• 7.0.0-alpha.0.50
• 7.0.0-alpha.0.51
• ...
• 7.0.0-alpha.1 (tagged)
• 7.0.0-alpha.1.1
• 7.0.0-alpha.1.2
• ...
• 7.0.0-alpha.2 (tagged)
• 7.0.0-alpha.2.1
• 7.0.0-alpha.2.2
• ...
• 7.0.0-beta.1 (tagged)
• 7.0.0-beta.1.1
• 7.0.0-beta.1.2
• ...
• 7.0.0 (tagged)
• 7.0.0-alpha.0.1 (default pre-release identifiers)
• 7.0.0-alpha.0.2
• ...and so on

Unless you are using a pre-release scheme where alpha doesn't make sense as the first pre-release phase (even if you don't intend to release an alpha), I strongly recommend that you stick to the default alpha.0 pre-release identifiers built into MinVer, in which case adamralph/minver#169 will have no value to you.

[slang25]

Thanks @adamralph, I was aware it wouldn't get rid of NU1505, like you say that's a tradeoff between SemVer 2.0 and ye olde clients. We are used to releasing just -alpha<some incrementing number> or -beta<some incrementing number> so wanted to be able to control the prerelease identifier (as we didn't need an additional controlled number), but if there is good reasoning to adopt the default in MinVer then I'm up for that.

[adamralph]

@slang25 even if you change the default pre-release identifiers, MinVer will still add the height to any commit which is not tagged. That behaviour cannot be changed. When you tag, you can tag with whatever version number you like. 7.0.0-alpha<some incrementing number> will work just fine. MinVer doesn't care what your pre-release versioning scheme is. In that case, your versions will look something like this:

• 7.0.0-alpha.0.49 (49 commits above the last tag or root commit)
• 7.0.0-alpha.0.50
• 7.0.0-alpha.0.51
• ...
• 7.0.0-alpha001 (tagged)
• 7.0.0-alpha001.1
• 7.0.0-alpha001.2
• ...
• 7.0.0-alpha002 (tagged)
• 7.0.0-alpha002.1
• 7.0.0-alpha002.2
• ...
• 7.0.0-beta001 (tagged)
• 7.0.0-beta001.1
• 7.0.0-beta001.2
• ...
• 7.0.0 (tagged)
• 7.0.0-alpha.0.1 (default pre-release identifiers)
• 7.0.0-alpha.0.2
• ...and so on

The assumption with the commits between the tags, is that they will not be released (or perhaps just to a specialised CI NuGet feed, e.g. on MyGet). If you don't care about them, then you also don't care about the alpha.0 identifiers. That said, if you do want to release the CI builds to a special NuGet feed, I'm not sure what the order of precedence will between alpha.0 and alpha001. If alpha.0 comes first, then you're fine, but if the other way round, then you do indeed need to change the default pre-release identifiers (to alpha000 in the example above).

Alternatively, as you suggest above, you could immediately tag the commit following 7.0.0 with 7.0.0-alpha000 to "prime" the pre-release identifiers.

[adamralph]

Although having said all of the above, I recommend just switching to SemVer 2.0. 🙂 I'm not sure it's worrying about those older clients anymore.

[slang25]

Just to be clear, I was thinking this:

• 7.0.0-alpha.49 (49 commits above the last tag or root commit)
• 7.0.0-alpha.50
• 7.0.0-alpha.51
• ...
• 7.0.0-beta (tagged)
• 7.0.0-beta.1
• 7.0.0-beta.2
• ...and so on

I'm thinking now we should just adopt the default.

[adamralph]

@slang25 that proposal would work, so long as you don't mind either:

• releasing every commit, or
• having gaps in your alpha/beta/etc. release numbers

[slang25]

It's a valid concern.

I think there may already be a requirement for this library to be consumed by nuget 4.3 and up because it has a netstandard2.0 target (not sure if it's required only if you are needing the target).

I'd be interested to know if there are any builds you are seeing using a version of nuget lower than 4.3 and that could could currently update to the current JustSaying 7.x

Doing some extra reading, the package versioning we have is likely to be compatible with older clients than 4.3: https://github.com/NuGet/Home/wiki/SemVer-2.0.0-support
I'll do some testing later in the week if required.

[martincostello]

Without doing a full audit of everyone's TeamCity builds, I have no idea what proportion that would be. But my team owns three components that are still on JustSaying 4.

Attempts were made to update them recently but the .NET Standard issues with .NET Framework 4.x have made that...fraught.

[slang25]

Cool, I'll try to verify client compatibility

[slang25]

It appears to be fine for 7.0.0-alpha.0.66, and we'd only be pushing 7.0.0-alpha.0 to nuget.org, which again is fine.

It's only a problem if you wanted to experiment with a non-released build with your CI nuget client where you couldn't update the client.

[adamralph]

FWIW, to clarify a couple of things:

• You wouldn't push alpha.0 to NuGet. The first tag and release would be alpha.1. The interim builds, which have the height attached, are designed to have an earlier version than the next release. That is why they are alpha.0.{height} rather than alpha.1.{height}
• The versions with and without height have the same client requirements. It's the dot after the alpha that makes it SemVer 2.0 rather than the two numbers.

[slang25]

Thanks for the clarification @adamralph

[stale]

This issue has been automatically marked as stale because it has not had any recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

[adamralph]

I saw the notification from the stale app that it's going to close this PR soon. If there's anything more I can do to help get this merged, please let me know.

[slang25]

Thanks @adamralph. I've applied your suggestions, and am happy with the state of this PR.

@AnthonySteele @martincostello I find it hard to gauge in GitHub comments the consensus and feelings towards things like this, feel free to let me know if this is something you do or don't want to adopt 🙂

[martincostello]

If I'm honest, I'm currently ambivalent to this either way, though that's mainly because when I first tried it I couldn't get it to work the way I wanted.

We have our weekly JustSaying meeting in 20 minutes so we can discuss there, then feedback here afterwards.

[slang25]

Cool, say hello to everyone from me

[adamralph]

...when I first tried it I couldn't get it to work the way I wanted.

@martincostello out of interest, what problems did you face?

[martincostello]

@adamralph I think it was because I was getting numbers like alpha.0.x where I didn't want the zero, but couldn't seem to suppress it. Admittedly this was on a repo that at the time was brand new and had no tags, but I think because I couldn't work out to remove it I just stopped looking at it.

[adamralph]

@martincostello the zero effectively means "there isn't yet an alpha". When you want to release your first alpha, you'll tag a commit with something like "1.0.0-alpha.1", indicating "alpha 1", i.e. the first alpha.

[adamralph]

This can now be 1.0.0-rc.1

[brainmurphy]

@slang25 Have merged 👍 Can you submit a PR to update MinVer to latest?