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

make the bootstrapper search harder for the paket.dependencies file #2384

Merged
merged 4 commits into from
Jun 2, 2017

Conversation

matthid
Copy link
Member

@matthid matthid commented Jun 1, 2017

like paket is doing it.

@vbfox please review. In particular I don't like how we now parse the command line twice in order to set the verbosity (such that we can trace on errors when finding the dependencies file). A better idea is welcome.

I guess I still need to add a test for this (I'm first trying to get the FAKE build green again).

@matthid matthid merged commit 96d8f11 into master Jun 2, 2017
@vbfox
Copy link
Contributor

vbfox commented Jun 2, 2017

👍 on the LocateDependencies change for me that the annoying thing about the boostrapper: It does some of what paket does but we can't share the code

On the trace I don't see the double parsing (Might have read too fast) and having a way to early-log is good.

@@ -23,9 +23,13 @@ static void Main(string[] args)
Console.CancelKeyPress += CancelKeyPressed;

var fileProxy = new FileSystemProxy();
var optionsBeforeDependenciesFile = ArgumentParser.ParseArgumentsAndConfigurations(args, ConfigurationManager.AppSettings,
Environment.GetEnvironmentVariables(), fileProxy, Enumerable.Empty<string>());
ConsoleImpl.Verbosity = optionsBeforeDependenciesFile.Verbosity;
Copy link
Member Author

Choose a reason for hiding this comment

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

@vbfox This is what I mean, we call ParseArgumentsAndConfigurations again below with an additional argument (which is Empty here).

I'm pretty sure we could refactor this somehow, but I couldn't come up with a nice way without changing basically everything.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the check for the environment variable should be done directly at that level without calling the ArgumentParser.

Accepting that the only way to verbose-log the early/configuration part of the bootstrapper is the environment variable.

@matthid matthid deleted the fix_bootstrapper_finding_dependencies branch June 15, 2017 22:11
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.

2 participants