-
Notifications
You must be signed in to change notification settings - Fork 654
Configuration Updates #341
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
Configuration Updates #341
Conversation
@@ -98,36 +90,16 @@ public override bool Execute() | |||
{ | |||
try | |||
{ | |||
CachedVersion versionAndBranch; | |||
Tuple<CachedVersion, GitVersionContext> versionAndBranch; |
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.
Not a massive fan of this, but it is a temporary hack to allow me to keep the MSBuild tasks in a working and compatible state.
With the Context being responsible for calculating effective configuration it needs to be exposed with the version for use in the variable provider.
As we get further down this refactoring hopefully more of this can all move into Core
I like the idea of having a merged However I would like to have a more dynamic solution here with reflection. Instead of calculating every property one by one in |
Could do. It would take 3 things, Config (which is the top level config), BranchConfig and EffectiveConfiguration (which is the defaults/fallback for everything). Will give that a go |
007e466
to
952659f
Compare
@yannisgu I had a crack at the more dynamic effective configuration, at the moment I think I am happy to hard code it. At least until we get this closer to the end state |
Yeah looks good 👍 |
…uration and trying to remove places ignoring configuration
8101e16
to
b9b2a2b
Compare
@yannisgu this branch is probably useful to you when I finish it. I think without the core configuration changes contained in this PR it will make updating the branch finders much harder (I think that is also why most of your tests are failing).
Thoughts on the direction in this PR?