-
Notifications
You must be signed in to change notification settings - Fork 654
Implemented configuration override for #826 #829
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
Conversation
I'm not really used to GitHub workflow, so I'm also not sure how to link this with #826 |
@labmonkey42 bu including #826 in your last post, GitHub took care of that mapping for you 👍 |
@labmonkey42 one thing I would say about your PR (note, I haven't had a chance to look at it in depth) would be that it is in your best interests to create a branch to do your work on. In all likelihood anytime you create a PR for any project, there is likely to be some feedback and modifications required. Simply put, this is much easier to do if the initial PR work is done on it's own branch. |
@gep13 thanks for the guidance. I thought about creating a branch here in this repo but I figured forking it would allow me to submit a PR without clogging your repo with another branch. |
@labmonkey42 forking is definitely the first part of the process, but a separate branch is also a standard work flow. For example, let's say our master branch is updated with the latest code from another scope of work, which impacts on your work. You would be able to pull in those changes to your forked master branch, and from there rebase your branch, sorting any merge conflicts. After that, pushing to your branch then automagically updates your PR branch. Then, once pulled in, our repo doesn't actually "see" your branch, so it doesn't clog anything up. |
@gep13 I see, thanks for the tip! |
@labmonkey42 FYI here is the suggested way to work in GitHub projects documented: https://guides.github.com/introduction/flow/ |
@@ -25,6 +25,7 @@ public static void WriteTo(Action<string> writeAction) | |||
eg /output json /showvariable SemVer - will output `1.2.3+beta.4` | |||
/l Path to logfile. | |||
/showconfig Outputs the effective GitVersion config (defaults + custom from GitVersion.yaml) in yaml format | |||
/overrideconfig Overrides GitVersion config values inline (semicolon-separated key value pairs e.g. /overrideconfig:tag-prefix=Foo) |
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.
It might be worth mentioning that only tag-prefix is supported at the moment?
…onfiguration directive supported by /overrideconfig
Does this need additional work before approval? Are we awaiting further feedback? As I'm not all that familiar with GitHub flows I'm just wondering what are the next steps. |
Thanks @labmonkey42 The delay was not caused by you :) |
Great, thanks for the update :) |
I've tried to implement this in an extensible manner such that other configuration options could be overridden with (e.g. /overrideConfig:foo=bar;tagPrefix=Foo.Data) but at this time only tagPrefix is parsed.
Feedback is certainly welcome.