Added logging message for tasks that fail without error#8125
Added logging message for tasks that fail without error#8125benvillalobos merged 5 commits intodotnet:mainfrom
Conversation
|
|
|
|
| // If is allowed to fail without error | ||
| if (be is IBuildEngine7 be7 && be7.AllowFailureWithoutError) | ||
| { | ||
| taskLoggingContext.LogComment(MessageImportance.Normal, "TaskReturnedFalseButDidNotLogError", _taskNode.Name); |
There was a problem hiding this comment.
Fit-and-finish question (@baronfel, especially): This logs with the MSB4181: prefix. Generally, we try not to have "error codes" for message-level logging. But here, it's kind of a warning, if you squint really hard. Should we strip the prefix, or leave it as-is?
There was a problem hiding this comment.
The normal case is hopefully non-error (they rerouted the error(s) appropriately, so it's just an informational message they don't care about). Most of our other messages are similar to this in that they might indicate an error but usually don't, so I'd vote for stripping the prefix, personally, but it's reasonable to leave it to baronfel.
There was a problem hiding this comment.
Most of our other messages are similar to this in that they might indicate an error but usually don't
I don't think I agree with this. We have a lot of messages that are fully neutral like "Started building target", and that feels like a very solid majority of logging in a normal build to me.
There was a problem hiding this comment.
I think of your example as "might indicate an error but usually don't"; if that target should not be building (or should not be building yet), it's indicating an error...but it's normally innocuous.
Specific version was added to avoid conflicts with newest internal version of dotnet and VS. Removed for PR
Co-authored-by: Rainer Sigwald <raines@microsoft.com>
| // If is allowed to fail without error | ||
| if (be is IBuildEngine7 be7 && be7.AllowFailureWithoutError) | ||
| { | ||
| taskLoggingContext.LogComment(MessageImportance.Normal, "TaskReturnedFalseButDidNotLogError", _taskNode.Name); |
There was a problem hiding this comment.
The normal case is hopefully non-error (they rerouted the error(s) appropriately, so it's just an informational message they don't care about). Most of our other messages are similar to this in that they might indicate an error but usually don't, so I'd vote for stripping the prefix, personally, but it's reasonable to leave it to baronfel.
| // If is allowed to fail without error | ||
| if (be is IBuildEngine7 be7 && be7.AllowFailureWithoutError) | ||
| { | ||
| taskLoggingContext.LogComment(MessageImportance.Normal, "TaskReturnedFalseButDidNotLogError", _taskNode.Name); |
There was a problem hiding this comment.
Most of our other messages are similar to this in that they might indicate an error but usually don't
I don't think I agree with this. We have a lot of messages that are fully neutral like "Started building target", and that feels like a very solid majority of logging in a normal build to me.
Fixes #6633
Context
Changes Made
Added check for tasks that fail without error messages, so that a normal priority message is shown.
Testing
Extended existing tests to validate new behavior. Also validated that in dotnet test scenarios, the new message is not visible.
Notes