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

[Fix #1187] Only write to STDERR if an error happened #1210

Merged
merged 1 commit into from
Apr 9, 2016
Merged

[Fix #1187] Only write to STDERR if an error happened #1210

merged 1 commit into from
Apr 9, 2016

Conversation

0x53A
Copy link
Contributor

@0x53A 0x53A commented Apr 9, 2016

Fixes #1187

Before: If the build was successfull, it would still write an empty line to STDERR, causing issues with powershell.

Now: it only writes if the error message is not empty.

@forki
Copy link
Member

forki commented Apr 9, 2016

Ah very good!

@forki forki merged commit bb157b3 into fsprojects:master Apr 9, 2016
@forki
Copy link
Member

forki commented Apr 9, 2016

Thanks for taking care of this.

@0x53A
Copy link
Contributor Author

0x53A commented Apr 9, 2016

Great, thank you! I see there is already a new nuget, so I can test it on monday at work.
I absolutely love CI and continuous deployment!

@matthid
Copy link
Member

matthid commented Apr 10, 2016

I'm not sure this is the correct fix (don't get me wrong I think the patch is a good idea anyway). My understanding of the code-base is that traceFAKE tries to write into the std-err stream but only in environments which will not cause the build to fail (otherwise it will write to regular std-out).

The logic for this is https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/TraceListener.fs#L87
So while you fixed one traceFAKE call you might run into the same trouble again in the future in another place where traceFAKE is used. So the question is can we detect if we are in an $ErrorActionPreference = true environment?

Just one note traceFAKE is not used that much throughout the codebase but traceImportant has the same behavior as it is defined almost the same way (https://github.com/fsharp/FAKE/blob/master/src/app/FakeLib/TraceHelper.fs#L49) and is used on quite some places.

Furthermore, I consider that PowerShell must be destroyed.

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.

3 participants