-
Notifications
You must be signed in to change notification settings - Fork 585
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
List targets quietly #1953
List targets quietly #1953
Conversation
Result after this change:
which any completion script could easily skip the first line of. |
Added sorting of the target names, but it's harder to test that because my script is using proper nugets. |
@@ -611,7 +611,8 @@ let prepareAndRunScript (config:FakeConfig) : RunResult = | |||
let retrieveHints (config:FakeConfig) (runResult:Runners.RunResult) = | |||
match runResult with | |||
| Runners.RunResult.SuccessRun _ -> [] | |||
| Runners.RunResult.CompilationError err -> [ |
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.
Actually that syntax will no longer yield a warning with next F# version ;)
src/app/Fake.netcore/Program.fs
Outdated
@@ -156,7 +156,7 @@ let runOrBuild (args : RunArguments) = | |||
let result = | |||
match runResult with | |||
| Runners.RunResult.SuccessRun warnings -> | |||
if warnings <> "" then | |||
if warnings <> "" && args.VerboseLevel.PrintNormal then | |||
traceFAKE "%O" warnings |
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.
Actually shouldn't this print into standard-error anyway? So your tool can just ignore stderr?
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.
that's a good point, let me try redirection and see what I get
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.
Yes you're right of course :D
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.
we can roll back this change then, I think.
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.
Yes, this basically means it already works are you are not blocked, correct (besides the sort)?
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.
yes, and I may not even need that (in zsh's completion, see image below). So maybe just rollback the whole thing?
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.
Just remove that line change, please
Thanks! I think travis is just choking right now as all builds seem to be red right now. |
This PR fixes a build break I got while building the repo, and silences the output of the warnings if the
RunContext
verbosity is set to Silent.