search

nunit / NUnit.System.Linq

Partial implementation of System.Linq for use with NUnit's .NET 2.0 builds
MIT License
1 stars 4 forks source link

Add GitVersionTask #6

Closed asbjornu closed 8 years ago

asbjornu commented 8 years ago

This PR adds GitVersionTask to version the assembly. Fixes #4.

asbjornu commented 8 years ago

To ensure that this works on Linux, I propose we set up a Travis build. I can do a PR for that and rebase once it's merged. Sound good, @CharliePoole?

rprouse commented 8 years ago

@asbjornu, a Travis build would be good. I will review this when I am at a computer.

CharliePoole commented 8 years ago

Failure because nuget packages are not installed. We would normally do this in the InitializeBuild task of the build.cake script. I generally do most things in scripts rather than relying on msbuild magic. :-) This build previously had no package dependencies so we should add a NuGetRestore call at the start of InitializeBuild.

I'm not seeing where we tell GitVersion how to create version numbers. Isn't there a config file?

WRT travis, we should definitely set that up at some point. We have it for all our other builds but this one is purely internal and we only make use of the packages. We will need it for the Debian packagers though, since they require building everything from source.

CharliePoole commented 8 years ago

@rprouse I'd like to use this project to set the "style" for how we use GitVersion. I'm not entirely convinced that GitVersionTask is the way to go but I figured that once @asbjornu got it set up, we could review and talk about alternatives. The choices seem to be GitVersion.exe, the cake GitVersion tool and GitVersionTask. Of course, we could try different approaches on different projects too.

asbjornu commented 8 years ago

@CharliePoole I've fixed the build problem, but as I want to use GitVersion to give AppVeyor its build number, I'm trying to work out the details in Cake's channel on Gitter. I'll keep on adding commits to this as I go and can rebase and squash in the end, if necessary.

With regards to how GitVersion creates version numbers; it does so by convention. If we need to deviate from the defaults, we can add a gitversion.yml file in the root of the repository to configure and customize the versioning scheme. I suggest going by the defaults and only adding that once we see the need for it.

CharliePoole commented 8 years ago

@asbjornu Once you get this working and we understand what it does, we can figure out further steps to take. This is a good project to start with because we are the only consumers of the packages. The assembly is included in our public NUnit package rather than depending on this package.

CharliePoole commented 8 years ago

@asbjornu Of course, we already set AppVeyor's build number in the script. That will have to be removed or replaced.

rprouse commented 8 years ago

I've merged the Travis build PR. You should pull in latest from master to get Travis building here too.

CharliePoole commented 8 years ago

Can you explain what is happening to AssemblyInfo.cs and when? Will it happen differently when somebody builds using VS versus doing it on AppVeyor?

asbjornu commented 8 years ago

@CharliePoole Nothing is happening with the checked-in AssemblyInfo.cs. It is not being touched by GitVersion. Instead, an AssemblyInfo_<random>.g.cs that contains the Assembly*Version attributes with the correct semantic version number is being generated compile-time. I like this better than using gitversion.exe and patching existing source, since that leads to hard questions like:

  1. Should the altered source files with the correct version number be committed?
  2. Won't the commit of the version number increase the version number?
  3. Since running gitversion.exe is optional, what will the version number of builds done directly in Visual Studio be?
  4. How does Visual Studio builds correlate to builds done by the continuous integration server?

With GitVersionTask automatically generating the version information compile-time, all of the above questions are moot:

  1. There is no version information in the source code. The version information is the tree of the SCM, as it should be.
  2. No, since there is no version information in the source code.
  3. GitVersionTask is executed compile-time, so it's not optional. You have to generate a fresh version number every time you compile.
  4. A build on the same commit in the same branch will generate the exact same version number regardless of who or where the build is performed. Inside Visual Studio, on the command line with a build script, in the continuous integration server; it will all lead to the same version number.

I hope this clarifies things. If not, please let me know! :smiley:

asbjornu commented 8 years ago

The Travis build is failing due to GitTools/GitVersion#890 as well as cake-build/cake#1021. We can either wait for those pull requests to be merged and released or we can exclude GitVersion in the Cake script (but not in MSBuild) on Unix for now. I'm trying to figure out why AppVeyor is failing and will continue that quest tomorrow.

CharliePoole commented 8 years ago

Yes, I was trying to suggest earlier that we not do this for this PR. We have no immediate need to build this project on Linux so we would only be doing it to prove it's possible.

OTOH all our other projects actually have to build on Linux.

asbjornu commented 8 years ago

@CharliePoole Yea, I'll ignore the Travis failures for now. Did my description of GitVersion above make sense? Questions? :smiley:

CharliePoole commented 8 years ago

@asbjornu Yes, your explanations make sense. I can see the advantage of not having any version info in the source code.

Looks like you are building now and did a rebase. I'll go through and re-review the code.

asbjornu commented 8 years ago

@CharliePoole Thanks for the review. Now that cake-build/cake#1021 is merged and GitTools/GitVersion#890 will hopefully be fixed by next week, it looks like we can have a working Unix build in not too long. Otherwise, I consider this PR done.

CharliePoole commented 8 years ago

I'll merge in the morning. I think I still had an outstanding question to you about how to tailor this to our own work practices, packaging standards, etc. but i can do that separately after the merge.

asbjornu commented 8 years ago

@CharliePoole Awesome! :smiley: I'm happy to answer any question anywhere you'd like. If there's anything you'd like to change about the current version number, please let me know and I'll add configuration to customize it.

CharliePoole commented 8 years ago

I guess the problem I see is that we no longer have a simple statement of how versions are generated and packages are labeled. Previously, you could look at the cake file and see code how we generate the code for version number and package label depending on whether there is a tag, whether it's a PR or not, whether the name of the branch starts with "release",...

Now that's all up to internal settings rather than being visible, which means nobody can work on the script without a knowledge of GitVersion internal settings. So I think we want to make the configuration explicit by generating the file, make it match our existing standards and then figure out how we may want to change it.

We do want to change it and it's OK if this particular repo is non-standard for a short while - that's why I picked it as the one to start with.

So I'll get the process going by merging this and then create a new issue for tailoring it.