-
Notifications
You must be signed in to change notification settings - Fork 651
Config file updates #302
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
Conversation
d05f1b3
to
1589271
Compare
Think this is ready to review. The only other thing I was thinking about dropping into config file was |
@@ -7,9 +8,11 @@ public abstract class RepositoryFixtureBase : IDisposable | |||
{ | |||
public string RepositoryPath; | |||
public IRepository Repository; | |||
Config configuration; |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DOne
@JakeGinnivan should this: Be updated to use |
@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? |
…Version class. This is done elsewhere
4d57b87
to
81779f7
Compare
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 |
|
@JakeGinnivan did you see this comment: |
Did see it, thought it was fixed. My bad. Done |
Nice one! This looks good to me. Anyone else want to take a look over it? |
wow 76 files changed. isnt that "all the files" :) |
@@ -1,4 +1,26 @@ | |||
public class Config | |||
namespace GitVersion.Configuration |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
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. |
Happy to roll this into 2.0.0 and fix a whole bunch of other issues. |
Should be good to go, bumped next milestone to be 2.0.0 |
It gets a |
Adds support for GitVersionConfig.yaml in GitVersion.exe and adds support for develop-branch-tag and release-branch-tag configuration options
Fixes #277