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

Config file updates #302

Merged
merged 17 commits into from Nov 17, 2014
Merged

Conversation

JakeGinnivan
Copy link
Contributor

Adds support for GitVersionConfig.yaml in GitVersion.exe and adds support for develop-branch-tag and release-branch-tag configuration options

Fixes #277

@JakeGinnivan JakeGinnivan added this to the 1.4.0 milestone Nov 15, 2014
@JakeGinnivan JakeGinnivan force-pushed the ConfigFileUpdates branch 2 times, most recently from d05f1b3 to 1589271 Compare November 15, 2014 23:16
This was referenced Nov 15, 2014
@JakeGinnivan
Copy link
Contributor Author

Think this is ready to review. The only other thing I was thinking about dropping into config file was next-version which will replace NextVersion.txt. But that should be another PR

@@ -7,9 +8,11 @@ public abstract class RepositoryFixtureBase : IDisposable
{
public string RepositoryPath;
public IRepository Repository;
Config configuration;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public? private? internal?

Small point, but should still be declared? Also, naming convention on variable name is inconsistent.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOne

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

@JakeGinnivan aside from a couple questions/comments, this looks good to me!

Is it worth shipping a default GitVersionConfig.yaml file, with the default values for each property?

@JakeGinnivan
Copy link
Contributor Author

Just rebased and fixed up new places which need tags cleaned up. Think all of your comments should be sorted.

As for default file. Lets create an issue to track that separately

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

As for default file. Lets create an issue to track that separately

#305

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

Just rebased and fixed up new places which need tags cleaned up. Think all of your comments should be sorted.

@JakeGinnivan did you see this comment:

#302 (comment)

@JakeGinnivan
Copy link
Contributor Author

Did see it, thought it was fixed. My bad. Done

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

Nice one!

This looks good to me. Anyone else want to take a look over it?

@SimonCropp
Copy link
Contributor

wow 76 files changed. isnt that "all the files" :)

@@ -1,4 +1,26 @@
public class Config
namespace GitVersion.Configuration
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the .Configuration namespace get us anything?

It will basically obscure any classes in that namespace. is this by design?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, just moved the files and r# added the namespace. Can put back to root?

@SimonCropp
Copy link
Contributor

so will this result in a major version bump?

@gep13
Copy link
Member

gep13 commented Nov 16, 2014

so will this result in a major version bump?

I thought that as well. At the minute, the next milestone is set at 1.4.0 (i.e. a bump in the minor version) but given the amount of moving parts that are changing, I think it deserves a major bump.

@JakeGinnivan
Copy link
Contributor Author

Happy to roll this into 2.0.0 and fix a whole bunch of other issues.

@JakeGinnivan
Copy link
Contributor Author

Should be good to go, bumped next milestone to be 2.0.0

@gep13
Copy link
Member

gep13 commented Nov 17, 2014

It gets a :shipit: from me.

@SimonCropp
Copy link
Contributor

pull the trigger

JakeGinnivan added a commit that referenced this pull request Nov 17, 2014
@JakeGinnivan JakeGinnivan merged commit 637c91c into GitTools:master Nov 17, 2014
@JakeGinnivan JakeGinnivan deleted the ConfigFileUpdates branch November 17, 2014 08:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add config file support to GitVersionExe
3 participants