[WIP] switch to MinVer #1484

adamralph · 2018-11-01T19:34:37Z

I hope you don't mind me sending this PR as a test of the MinVer alpha package. This is not intended to be merged, I just wanted to ensure the package works for FIE in the context of the Appveyor build.

Of course, any feedback is more than welcome.

adamralph · 2018-11-01T19:35:52Z

tools/FakeItEasy.Build/Program.cs

+            ////Target(
+            ////    "pack-nuspecs",
+            ////    DependsOn("outputDirectory", "get-version"),
+            ////    () => Run(ToolPaths.NuGet, $"pack {AnalyzerMetaPackageNuspecPath} -Version {version} -OutputDirectory {OutputDirectory} -NoPackageAnalysis"));


There are ways of making this work without using nuget.exe and passing the version explicitly, but for the purpose of this test, I just commented it out. Also, IIRC you were considering getting rid of this meta-package.

adamralph · 2018-11-01T19:52:41Z

Note that the version is 4.1.0-alpha004.137 because the last tag in the history of develop is 4.1.0-alpha.4 on 5bb6d62, which is 137 commits back. MinVer completely ignores branches, so you have to "tell" it the current version range of develop, e.g. by pushing a tag like 4.10.0-alpha.0, or even just 4.10.0-0.

thomaslevesque · 2018-11-01T22:15:27Z

Nice, @adamralph!
MinVer looks pretty cool, and much simpler to understand than GitVersion.

Note that the version is 4.1.0-alpha004.137 because the last tag in the history of develop is 4.1.0-alpha.4 on 5bb6d62, which is 137 commits back.

Just to make sure I understand: when we merge the next release branch to master and tag 4.10.0 on master, this will generate a 4.10.0 version, right?

adamralph · 2018-11-01T22:22:39Z

Just to make sure I understand: when we merge the next release branch to master and tag 4.10.0 on master, this will generate a 4.10.0 version, right?

@thomaslevesque correct. More details here.

BTW, regarding this:

MinVer completely ignores branches, so you have to "tell" it the current version range of develop, e.g. by pushing a tag like 4.10.0-alpha.0, or even just 4.10.0-0.

We're also working on a feature where you can tell MinVer the minimum major and minor of the mainline integration branch (in your case develop) via an MSBuild property, rather than pushing an "artificial" tag.

adamralph · 2018-11-03T15:36:08Z

We're also working on a feature where you can tell MinVer the minimum major and minor of the mainline integration branch (in your case develop) via an MSBuild property, rather than pushing an "artificial" tag.

As promised, in alpha 12: 😉

That's proven the alpha for FakeItEasy so I'm closing this. Thanks for hosting this test! 😄

FYI I'm performing the final round of alpha testing now, after which I'll release the first beta. @FakeItEasy/owners would you be interested in switching to MinVer while it's in beta? Or would you rather wait for RC/RTM?

Of course, I'm being presumptuous. If you prefer to stick with GitVersion then I'll shut up. 😁

blairconrad

Thanks, @adamralph. I like the idea of a simpler versioner (GitVersion never sat that well with me), but am not wild about having to set a variable to say what our preferred major/minor version is. I suppose if we were using GitHubFlow instead of GitFlow, this would not be an issue? Or if we recreated develop after each release…

blairconrad · 2018-11-03T16:05:34Z

src/CommonAssemblyInfo.cs

 using System.Reflection;
 using System.Resources;
 using System.Runtime.InteropServices;

-[assembly: AssemblyCompany("Patrik Hägne")]
-[assembly: AssemblyCopyright("Copyright (c) FakeItEasy contributors. (fakeiteasyfx@gmail.com)")]


Why'd you remove these lines?

@blairconrad it's because I had to remove <GenerateAssemblyInfo>false</GenerateAssemblyInfo> from the projects because the assembly info should come from the project properties rather than a VersionInfo.cs generated by GitVersion.

thomaslevesque · 2018-11-03T16:48:01Z

but am not wild about having to set a variable to say what our preferred major/minor version is

Agreed. Without this, it generates the wrong version when on develop, but it's already the case anyway, as you can see here.

adamralph · 2018-11-03T17:01:37Z

...(I) am not wild about having to set a variable to say what our preferred major/minor version is.

@blairconrad @thomaslevesque that's the trade off for simplicity. MinVer doesn't know anything about branches. You have to tell it which version range the mainline branch is for, if it's not the version implied by the latest tag. I tried to think of all possible ways of avoiding this, but all paths lead back to branch inspection.

Upon initial investigation, it may seem we only need a few simple branch checks. Believe me, it's an enormous can of worms best left sealed. I did spike another approach which did inspect branches. It was fiendishly complicated. I never felt completely comfortable that I'd covered all the scenarios. Not to mention that I'd only implemented it for one branching model, Release Flow. To support other branching models I would have had to write a whole load more logic and configuration options. "Can of worms" doesn't do it justice. I hate to think how much complication and maintenance burden this adds to GitVersion.

I suppose if we were using GitHubFlow instead of GitFlow, this would not be an issue? Or if we recreated develop after each release…

As I explained above, branches make no difference. There's no getting away from having to tell MinVer the major minor range of the mainline branch.

Bear in mind that there is another option: you can push a "dummy" tag, e.g. 4.10.0-alpha.0, which does not represent a release. It's only purpose is to tell MinVer which version range the branch is in. The pre-release identifiers are meaningless, but the version has to precede any real tag you intend to use. The "lowest" possible tag would be 4.10.0-0. Personally, I prefer the MinVerMinimumMajorMinor method, since then I know that all tags represent releases.

...it's already the case anyway, as you can see here.

I'm not sure why that's happening. I guess we'd need to inspect what the (fiendishly complicated 😉) GitVersion algorithm is doing to figure it out. But to fix it, I suspect you'll have to put the correct version in GitVersion.yml, and managing the version in that file is equivalent to managing MinVerMinimumMajorMinor. This is exactly what we do in our repos that use GitVersion, e.g. https://github.com/Particular/NServiceBus.RabbitMQ/blob/e24aa90a0a3113c6801a785a97e922aed4686770/GitVersion.yml#L2. That effectively removes this perceived downside of switching to MinVer.

blairconrad · 2018-11-03T18:00:52Z

but am not wild about having to set a variable to say what our preferred major/minor version is

Agreed. Without this, it generates the wrong version when on develop, but it's already the case anyway, as you can see here.

The PR builds currently have bad versions, but the builds on develop are better behaved: build 4.10.0-alpha063.nupkg.
Would MinVer do the same thing?

I suppose if we were using GitHubFlow instead of GitFlow, this would not be an issue? Or if we recreated develop after each release

As I explained above, branches make no difference. There's no getting away from having to tell MinVer the major minor range of the mainline branch.

But if we were using GitHubFlow, we'd have a recent tag in the history, would we not? So the new builds on master would have version <recent tagged version>+<some minver stuff>.

adamralph · 2018-11-03T19:57:46Z

Would MinVer do the same thing?

@blairconrad I'm not really sure what you mean. The algorithm that MinVer uses is described here: https://github.com/adamralph/min-ver#algorithm

But if we were using GitHubFlow, we'd have a recent tag in the history, would we not? So the new builds on master would have version <recent tagged version>+<some minver stuff>.

When MinVer sees an RTM tag in the history, it can't tell if you're now working on the next patch or the next minor. It has to assume patch, since if it assumed minor, it would start producing versions higher than the patch. If you're working on the next minor, you have to tell it, either by pushing a tag or altering MinVerMinimumMajorMinor. Again, it's probably best to read the algorithm and then it will become clear.

blairconrad · 2018-11-03T23:30:48Z

Yeah, I'd read the algorithm. It wouldn't be branches that would make the difference under GitHubFlow, but the reachability of the tags.
If we were using GitHubFlow (no develop, only master), the versions would work out great on the PR builds or even untagged builds done on master, since the most recent release tag would be reachable.
So in our case today, we'd get build numbers like 4.9.1+1-alpha.0.65, not unlike how GitVersion uses a more complicated algorithm and generated 4.10.0-alpha065 for the most recent merge to develop. Of course GitVersion accomplishes it because it understands the branches.

I'm put off by the MinVer environment variables and phantom tags, and would prefer not to switch to MinVer while using GitFlow, unless we started recreating develop (equivalently, rebasing onto master) after every release.

thomaslevesque · 2018-11-04T10:36:30Z

unless we started recreating develop (equivalently, rebasing onto master)

Or maybe just merging master into develop after a release? This would make the tags reachable.

blairconrad · 2018-11-04T11:22:12Z

Or maybe just merging master into develop after a release? This would make the tags reachable.

Do you mean an ugly --no-ff merge that makes the graph so complicated?

or a fast-forward merge that's equivalent to a rebase?

I could get behind the latter. Oh, heck. I'll mention my other option. We could drop GitFlow. I've not once seen any benefit.

adamralph · 2018-11-04T13:11:52Z

I wouldn't rely too much on branching solving the problem. Regardless of your branching pattern and the reachable tags, MinVer can't automatically know whether your working on the next patch, the next minor, or the next major.

E.g. Using GitHub Flow, you tag master as 4.10.0 and release. You then keep committing to master (or whatever branch, it's not important). Those commits will be built as 4.10.1-alpha.0.{height}. If you're working on the next patch, that's fine. If you're working on 4.11.0 or 5.0.0, you have to tell MinVer, so that it builds as 4.11.0-alpha.0.{height} or 5.0.0-alpha.0.{height}.

But I would argue that GitHub Flow is not really what you want, since then the only releasable thing is master, so if you're half way through 4.11.0 or 5.0.0 work, you can't go back and patch 4.10.0. For that reason, I would consider Release Flow, which is really just a slight variant from GitHub Flow. In fact, you only need to create a release-4.10 branch for fixing pre-release versions after you've decided that master has moved past 4.10.0 (usually at the 4.10.0 RC stage). If you didn't have to do that, there's no reason to create a release-4.10 branch before releasing 4.10.0. In that case, Release Flow looks just like GitHub Flow until you need to patch.

When using Release Flow, you would never be working on the next patch in master, so after releasing 4.10.0 you would immediately tell MinVer that you're now working on 4.11.0 or 5.0.0. If you need to patch 4.10.0 you go back to that tag, create a release-4.10 branch (if you haven't done so already) and commit to that branch. In that case you don't have to tell MinVer anything, since it is already assuming you are working on the next patch and building interim commits as 4.10.1-alpha.{height}.

In retrospect I think I've done a bad job so far of selling the concept and my explanations have made MinVer sound more complicated than it is. To make the whole thing easier to understand, I'm considering re-writing the README and renaming some of the input variables. I'll also reduce the env var/property synonyms to only those which actually make sense. Try and forget about "MinVer environment variables and phantom tags".

I'll try and re-phrase (as a dress rehearsal for the new README): In any given branch, you are working in a major-minor range. All your release versioning within that range (pre-releases, patches, etc.) is done just by tagging the repo. Now, if you haven't already released (either pre-release or RTM), then there's no tag which indicates the major-minor range. If you want your builds to be in that range before the first release, you tell MinVer the range via the MinVerMajorMinor property (not in environment variables or dummy tags, but in code). When you start work on a new major-minor range, change the MinVerMajorMinor property.

Of course, if you don't care about your builds being in correct range before the first release, then you don't need MinVerMajorMinor. You just install MinVer and start tagging away.

Note that this is equivalent to GitVersion with respect to major versions. GitVersion can infer a new minor range by inspecting the branch structure, but it can't possibly know that you've moved onto a new major unless you put that in the YAML file. If you don't do that, then GitVersion will also keep building as 4.x until you push a tag in the 5.0 range.

adamralph · 2018-11-04T17:15:56Z

@blairconrad @thomaslevesque FYI I've re-written the README with specific attention to a new Usage section.

As ever, your feedback is proving invaluable. I'm enormously grateful, regardless of whether or not you choose to use MinVer.

thomaslevesque · 2018-11-04T18:26:26Z

Do you mean an ugly --no-ff merge that makes the graph so complicated?

or a fast-forward merge that's equivalent to a rebase?

Yeah, I guess a fast-forward makes more sense.

Oh, heck. I'll mention my other option. We could drop GitFlow. I've not once seen any benefit.

Well, I'm not married to GitFlow, so why not. We've been applying it very loosely anyway (we never do hotfix branches, and the release branches never receive any commit, we just merge them to master directly). Maybe Release Flow would work for us.

@adamralph thanks for the detailed explanations.

Anyway, with out current workflow, we often don't know exactly what the next release will be; it depends on what we committed to develop since the last release. If there are only bug fixes, it's a patch, if there are non-breaking functional changes, it's a minor, and if there are breaking changes, it's a major. So it wouldn't really make sense to set MinVerMajorMinor in the project, since we just don't know. And it's probably not very important to have correct version numbers on develop, since we don't know yet what the next version should be.

adamralph · 2018-11-04T23:45:37Z

@thomaslevesque ah, so in your case, things are even simpler. If you used MinVer, all you'd need to do is push tags. You can ignore the entire conversation above about "phantom" tags, environment variables, and MSBuild properties. 😃

On the topic of workflows, both Git Flow and Release Flow assume that the mainline branch (develop or master respectively) is for new minors or majors only. There is no such thing as a "patch only" release from the mainline branch. Any bug fix that needs to be released before the next minor or major is patched directly onto the release branch (master or release-x.y respectively). If you never do that, you may as well go back to GitHub Flow. And the nice thing is, if you ever find that you do need to release a bug fix independently of the other work that's on master, you can just go back to the last RTM tag, create a release branch and go ahead and patch. Hey presto, you are now using Release Flow! As I said above, until that time comes, GitHub Flow and Release Flow are identical. And if you were using MinVer, it's only at that time that you might want to consider setting MinVerMajorMinor, but only if you care about having the correct version on builds before the next tag on master, which it doesn't seem you are too bothered about.

After learning about Release Flow, I'm no longer sure there's any use for Git Flow.

blairconrad · 2018-11-05T16:57:04Z

@thomaslevesque, well explained. I'd been assuming that @adamralph was on top of all this, but it's been a while, and there are so many flows in so many different projects, I shouldn't've. Sorry, @adamralph.

@adamralph, your workflow for releasing patch versions matches my thinking. In general, I think we'd just release as soon as possible off master, including whatever was in master at the time. If for some reason there was some unreleasable work in master (which hasn't been the case for a very long time), branching from the tag and releasing from there works for me, then merging into master.

For me, having current interim builds off master (assuming a GitFlowish workflow) look like 4.9.2-alpha.68 or similar is not a problem.

(Although at times having the SHA in there would be convenient, for example when I'm building locally and importing nupkgs into my benchmarks, but this is a nice-to-have, not a necessity. And if I understand things correctly, I might be able to get what I want by using the MINVER_BUILD_METADATA environment variable.)

blairconrad · 2018-11-05T17:04:03Z

I'm enjoying MINVER_BUILD_METADATA:

λ  $env:MINVER_BUILD_METADATA=(git rev-parse --short=12 HEAD ); dotnet build
Microsoft (R) Build Engine version 15.8.166+gd4e8d81a88 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 28.33 ms for D:\Sandbox\MinVerPlay\MinVerPlay.csproj.
  MinVer: Using { Commit: c366c4f, Tag: '1.0.0', Version: 1.0.0, Height: 1 }.
  1.0.1-alpha.0.1+78090161e63c

and after fixupping that last commit:

λ  $env:MINVER_BUILD_METADATA=(git rev-parse --short=12 HEAD ); dotnet build
Microsoft (R) Build Engine version 15.8.166+gd4e8d81a88 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 28.79 ms for D:\Sandbox\MinVerPlay\MinVerPlay.csproj.
  MinVer: Using { Commit: c366c4f, Tag: '1.0.0', Version: 1.0.0, Height: 1 }.
  1.0.1-alpha.0.1+512a239b282d

adamralph · 2018-11-05T17:16:50Z

Oh, nice idea!

BTW, I'm not sure if you already know, but Microsoft.SourceLink.GitHub can publish the repository info into the package nuspec if you switch on PublishRepositoryUrl.

The element in the nuspec ends up looking something like this:

<repository
    type="git"
    url="https://github.com/adamralph/bullseye.git"
    commit="fceca30fbab5d654bb64aaf01a0669064c0997d9" />

...and that shows up nicely in NuGet Package Explorer:

blairconrad · 2018-11-05T17:28:51Z

Thanks, @adamralph. I did not know this. It looks handy.
I've only been tangentially aware of SourceLink as "something we might want to switch to from PdbGit". Maybe that's completely off the mark…

adamralph · 2018-11-05T18:21:36Z

@blairconrad Sourcelink is very nice. You just install the package and it Just Works™.

Although I seem to remember there were some wrinkles in FakeItEasy related to the IL merging. Perhaps that was with GitLink?

blairconrad · 2018-11-05T20:15:41Z

It was with SourceLink. #1417.

thomaslevesque · 2018-11-05T20:58:02Z

I've only been tangentially aware of SourceLink as "something we might want to switch to from PdbGit".

It is. But it can also add the repo URL to the package metadata (which we could do manually anyway)

@blairconrad Sourcelink is very nice. You just install the package and it Just Works™.

Although I seem to remember there were some wrinkles in FakeItEasy related to the IL merging. Perhaps that was with GitLink?

#1417 (comment)

switch to MinVer

f7aa656

adamralph commented Nov 1, 2018

View reviewed changes

adamralph mentioned this pull request Nov 1, 2018

1.0.0 release adamralph/minver#16

Closed

46 tasks

upgrade to MinVer 1.0.0-alpha.12

43e0c85

adamralph closed this Nov 3, 2018

blairconrad reviewed Nov 3, 2018

View reviewed changes

adamralph mentioned this pull request Nov 3, 2018

Add FAQ about having to specify minimum major minor adamralph/minver#74

Closed

adamralph mentioned this pull request Nov 4, 2018

simplify inputs adamralph/minver#82

Merged

blairconrad mentioned this pull request Nov 5, 2018

Drop GitFlow #1487

Closed

13 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] switch to MinVer #1484

[WIP] switch to MinVer #1484

adamralph commented Nov 1, 2018

adamralph Nov 1, 2018 •

edited

Loading

adamralph commented Nov 1, 2018

thomaslevesque commented Nov 1, 2018

adamralph commented Nov 1, 2018

adamralph commented Nov 3, 2018

blairconrad left a comment

blairconrad Nov 3, 2018

adamralph Nov 3, 2018

thomaslevesque commented Nov 3, 2018

adamralph commented Nov 3, 2018

blairconrad commented Nov 3, 2018

adamralph commented Nov 3, 2018

blairconrad commented Nov 3, 2018

thomaslevesque commented Nov 4, 2018

blairconrad commented Nov 4, 2018

adamralph commented Nov 4, 2018

adamralph commented Nov 4, 2018

thomaslevesque commented Nov 4, 2018

adamralph commented Nov 4, 2018

blairconrad commented Nov 5, 2018

blairconrad commented Nov 5, 2018 •

edited

Loading

adamralph commented Nov 5, 2018

blairconrad commented Nov 5, 2018

adamralph commented Nov 5, 2018

blairconrad commented Nov 5, 2018 •

edited

Loading

thomaslevesque commented Nov 5, 2018

[WIP] switch to MinVer #1484

[WIP] switch to MinVer #1484

Conversation

adamralph commented Nov 1, 2018

adamralph Nov 1, 2018 • edited Loading

Choose a reason for hiding this comment

adamralph commented Nov 1, 2018

thomaslevesque commented Nov 1, 2018

adamralph commented Nov 1, 2018

adamralph commented Nov 3, 2018

blairconrad left a comment

Choose a reason for hiding this comment

blairconrad Nov 3, 2018

Choose a reason for hiding this comment

adamralph Nov 3, 2018

Choose a reason for hiding this comment

thomaslevesque commented Nov 3, 2018

adamralph commented Nov 3, 2018

blairconrad commented Nov 3, 2018

adamralph commented Nov 3, 2018

blairconrad commented Nov 3, 2018

thomaslevesque commented Nov 4, 2018

blairconrad commented Nov 4, 2018

adamralph commented Nov 4, 2018

adamralph commented Nov 4, 2018

thomaslevesque commented Nov 4, 2018

adamralph commented Nov 4, 2018

blairconrad commented Nov 5, 2018

blairconrad commented Nov 5, 2018 • edited Loading

adamralph commented Nov 5, 2018

blairconrad commented Nov 5, 2018

adamralph commented Nov 5, 2018

blairconrad commented Nov 5, 2018 • edited Loading

thomaslevesque commented Nov 5, 2018

adamralph Nov 1, 2018 •

edited

Loading

blairconrad commented Nov 5, 2018 •

edited

Loading

blairconrad commented Nov 5, 2018 •

edited

Loading