-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
CLI Commands: Automatically build the project #4602
Conversation
Reporter.Verbose.WriteLine("Build started...".Bold().Black()); | ||
|
||
var buildCommand = Command.Create( | ||
Path.Combine(Environment.GetEnvironmentVariable("DOTNET_HOME"), "bin", "dotnet" + Constants.ExeSuffix), |
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.
@davidfowl Good enough?
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.
Doh! DOTNET_HOME
is being removed in dotnet/cli#1478
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.
Seems like we should raise an error if DOTNET_HOME
is not defined?
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.
Sounds like we're going to assume it's on the path. (see dotnet/cli#1150) I'll update the PR.
if (args[i] == "-v" || args[i] == "--verbose") | ||
{ | ||
Environment.SetEnvironmentVariable(CommandContext.Variables.Verbose, bool.TrueString); | ||
args = args.Take(i).Concat(args.Skip(i + 1)).ToArray(); |
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.
If you find a -v
or --verbose
at say, i = 4
, then having re-calculated args
wouldn't you need to start at i = 4
again in order to look at whatever was originally at i = 5
but now is at i = 4
instead of just carrying on with the i++
in the loop?
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.
The loop breaks after this. We find the first one, process and remove it, then stop
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.
OK. Sounds fine so long as if there are multiple -v
or --verbose
flags it doesn't cause some strange error later on.
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.
They're swallowed gracefully by the parser. 🍸
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.
OK. Sounds good.
|
Part of #3925
Additional changes:
--verbose
anywhere