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

[WIP] Branch specific configuration #338

Closed
wants to merge 78 commits into from

Conversation

@JakeGinnivan
Copy link
Contributor

JakeGinnivan commented Jan 7, 2015

The goal of this PR is to remove the whole branch specific code and to enable a better "branch-configuration". To do this we need basically three things:

  • Branch-specific configuration: Done by adding a CurrentBranchConfig property to GitVersionContext
  • Implement different version calculation strategies like NextVersionConfigStrategy, MergeMessageVersionStrategy or LastTagVersionStrategy. For all branches the version is then calculated like this
var versionStrategies = new IVersionStrategy[] { new NextVersionConfigStrategy(), new MergeMessageVersionStrategy(), new LastTagVersionStrategy() };
var version = versionStrategies.Select(s => s.CalculateVersion(configuration, branchConfiguration)).Max();
  • Implement different variable providers (continous deployment => every commit has different NuGet Version, continous delivery) to enable following code:
var variables = variableProvider.Provide(branchConfiguration.Mode);

Outstanding tasks

  • Branch-specific configuration
  • Update variable provider
  • Fix broken tests
  • Legacy configuration error messages
  • Documentation - https://github.com/ParticularLabs/GitVersion/wiki/Branch-Specific-Configuration
  • Discuss: continuous deployment 4 part stable version not including 4th part if it is 0
  • Add useBranchNameAsTag rather than using "useBranchName" magic string. useBranchNameAsTag being true should cause the tag setting to be ignored.
Yannis Guedel and others added 6 commits Dec 9, 2014
@yannisgu
Copy link
Contributor

yannisgu commented Jan 8, 2015

@JakeGinnivan the description is kind of outdated. Maybe change it to something like this:

The goal of this PR is to remove the whole branch specific code and to enable a better "branch-configuration". To do this we need basically three things:

  • Branch-specific configuration: Done by adding a CurrentBranchConfig property to GitVersionContext
  • Implement different version calculation strategies like NextVersionConfigStrategy, MergeMessageVersionStrategy or LastTagVersionStrategy. For all branches the version is then calculated like this
var versionStrategies = new IVersionStrategy[] { new NextVersionConfigStrategy(), new MergeMessageVersionStrategy(), new LastTagVersionStrategy() };
var version = versionStrategies.Select(s => s.CalculateVersion(configuration, branchConfiguration)).Max();
  • Implement different variable providers (continous deployment => every commit has different NuGet Version, continous delivery) to enable following code:
var variables = variableProvider.Provide(branchConfiguration.Mode);
@yannisgu
Copy link
Contributor

yannisgu commented Jan 8, 2015

I started offline with this version finders.. Will try to quickly create a PR

@JakeGinnivan
Copy link
Contributor Author

JakeGinnivan commented Jan 8, 2015

No worries.

I have made a start on the variable provider stuff. I think we need to introduce an effective configuration which flattens everything out based on the current branch and apply fallbacks etc. Will give that a spin

@JakeGinnivan
Copy link
Contributor Author

JakeGinnivan commented Jan 8, 2015

I think breaking changes are a good idea. Mainly:

  • Config file changes. We could migrate like we do now, but could be complex. Maybe we need to introduce a fileVersion or something to the config file so we can version it properly (i.e upgrade to v3 of the config file to light up all these features).
  • Removing options from the MSBuild tasks and only support the configuration file.

What do yo think @SimonCropp @andreasohlund

@SimonCropp
Copy link
Contributor

SimonCropp commented Jan 8, 2015

not sure i like putting in a version number into the file. would prefer to gradually add options and give good errors when we obsolete old ones

+1 on removing MSBuild option support

@yannisgu
Copy link
Contributor

yannisgu commented Jan 9, 2015

This should be really easy to implement: Throw an exception in the old properties of Config and handle them nicely.

@JakeGinnivan
Copy link
Contributor Author

JakeGinnivan commented Jan 9, 2015

Am working on another PR which has a legacy config class, reads in everything legacy then provides migration information in a error message.

Sent from my Windows Phone


From: yannisgumailto:notifications@github.com
Sent: ý09/ý01/ý2015 07:59
To: ParticularLabs/GitVersionmailto:GitVersion@noreply.github.com
Cc: Jake Ginnivanmailto:jake@ginnivan.net
Subject: Re: [GitVersion] [WIP] Branch specific configuration (#338)

This should be really easy to implement: Throw an exception in the old properties of Config and handle them nicely.


Reply to this email directly or view it on GitHubhttps://github.com//pull/338#issuecomment-69304080.

@JakeGinnivan
Copy link
Contributor Author

JakeGinnivan commented Jan 9, 2015

Added a wiki page so we can write the doco as we go

https://github.com/ParticularLabs/GitVersion/wiki/Branch-Specific-Configuration

JakeGinnivan added 6 commits Jan 9, 2015
Throw error when user is using legacy configuration values
…uration and trying to remove places ignoring configuration
@JakeGinnivan
Copy link
Contributor Author

JakeGinnivan commented Feb 13, 2015

@andreasohlund there is one major downside with this approach, it understands merging and reachable versions.

Develop no longer tracks master to bump the version of develop. We also do not have any enforcing if you do GitFlow wrong. I think this is a simpler approach and should work fine.

         *  hotfix-1.2.1       -----------C--      
         *                    /              \     
         *  master           A----------------F-----H-------N
         *                    \                    / \     /
         *  hotfix-1.3.1       \                  /   ----L
         *                      \                /         \
         *  release-1.3.0        \        -D----G---        \
         *                        \      /          \        \
         *  develop                -----B----E-------I-----M--O--P
         *                                    \           /
         *  feature                            -------J-K-

In this case though when release-1.3.0 is merged into master, J used to bump to 1.4.0. This will no longer happen. If you rebase feature then it will bump to 1.4.0 because release-1.3.0 has been merged into develop.

Can you forsee any issues with this?

@andreasohlund
Copy link
Contributor

andreasohlund commented Feb 13, 2015

I can't see any major drawbacks, +1 for proceeding

On Fri, Feb 13, 2015 at 6:09 PM, Jake Ginnivan notifications@github.com
wrote:

@andreasohlund https://github.com/andreasohlund there is one major
downside with this approach, it understands merging and reachable
versions.

Develop no longer tracks master to bump the version of develop. We also do
not have any enforcing if you do GitFlow wrong. I think this is a simpler
approach and should work fine.

     *  hotfix-1.2.1       -----------C--
     *                    /              \
     *  master           A----------------F-----H-------N
     *                    \                    / \     /
     *  hotfix-1.3.1       \                  /   ----L
     *                      \                /         \
     *  release-1.3.0        \        -D----G---        \
     *                        \      /          \        \
     *  develop                -----B----E-------I-----M--O--P
     *                                    \           /
     *  feature                            -------J-K-

In this case though when release-1.3.0 is merged into master, J used to
bump to 1.4.0. This will no longer happen. If you rebase feature then it
will bump to 1.4.0 because release-1.3.0 has been merged into develop.

Can you forsee any issues with this?


Reply to this email directly or view it on GitHub
#338 (comment)
.

@JakeGinnivan JakeGinnivan deleted the BranchSpecificConfiguration branch Feb 13, 2015
@SimonCropp

This comment has been minimized.

Copy link
Contributor

SimonCropp commented on GitVersionCore/GitVersionContext.cs in 65d0c0c Feb 15, 2015

Should this be t.PeeledTarget() instead of t.Target

This comment has been minimized.

Copy link
Contributor Author

JakeGinnivan replied Feb 15, 2015

I only noticed that in something.. Will do a find/replace...

This comment has been minimized.

Copy link
Contributor Author

JakeGinnivan replied Feb 15, 2015

Just pushed a commit which fixed 3 instances of this

This comment has been minimized.

Copy link
Contributor

SimonCropp replied Feb 15, 2015

@nulltoken shouldnt PeeledTarget be something included in libgit?

This comment has been minimized.

Copy link
Contributor

SimonCropp replied Feb 15, 2015

@JakeGinnivan i actually asked rather then doing a commit since i wasnt sure :)

This comment has been minimized.

Copy link
Contributor Author

JakeGinnivan replied Feb 15, 2015

I'm not sure either, but it doesn't look like it would hurt :P

This comment has been minimized.

Copy link
Contributor

nulltoken replied Feb 15, 2015

@bording is working on making this happen (see libgit2/libgit2sharp#551) 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
You can’t perform that action at this time.