Fix regression with log file summary. #2110
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A change was (dotnet#1513) to allow for more control over log summary text. Specifically, the ask was to allow for the use of ErrorsOnly / WarningsOnly with Summary. That change also impacted the file log output and in some cases caused build summary to output to the log file when using ErrorsOnly. This change defaults to not showing the summary text in the FileLogger when ErrorsOnly or WarningsOnly is set (it can still be displayed with the Summary flag). This reverts behavior back to pre-VS2017 behavior and allows for an empty err/wrn file to be produced. Closes dotnet#1968
jeffkl
approved these changes
May 18, 2017
cdmihai
reviewed
May 18, 2017
| @@ -342,6 +343,36 @@ public void BasicNoExistingDirectory() | |||
| } | |||
| } | |||
|
|
|||
| [Theory] | |||
| [InlineData("warningsonly")] | |||
There was a problem hiding this comment.
What happens if both are set? :)
AndyGerlicher
force-pushed
the
emptyerrorlogfix2
branch
from
1b3ea8e
to
5caff1c
Compare
rainersigwald
approved these changes
May 18, 2017
|
|
| @@ -269,7 +274,7 @@ private void ApplyFileLoggerParameter(string parameterName, string parameterValu | |||
| /// <summary> | |||
| /// Encoding for the output. Defaults to UTF-8. | |||
| /// </summary> | |||
| private Encoding _encoding = Encoding.UTF8; | |||
| private Encoding _encoding = new UTF8Encoding(false); | |||
There was a problem hiding this comment.
You don't need to pass false because the default ctor of UTF8Encoding passes false already:
http://referencesource.microsoft.com/#mscorlib/system/text/utf8encoding.cs,65
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A change was (#1513) to allow for more control over log summary text.
Specifically, the ask was to allow for the use of ErrorsOnly /
WarningsOnly with Summary. That change also impacted the file log output
and in some cases caused build summary to output to the log file when
using ErrorsOnly.
This change defaults to not showing the summary text in the FileLogger
when ErrorsOnly or WarningsOnly is set (it can still be displayed with
the Summary flag). This reverts behavior back to pre-VS2017 behavior and
allows for an empty err/wrn file to be produced.
Closes #1968