Skip to content

Terminal logger enablement by /tlp:default and related telemetry#9119

Merged
rokonec merged 10 commits intodotnet:mainfrom
rokonec:rokonec/9063-skd-tl-defaults
Aug 16, 2023
Merged

Terminal logger enablement by /tlp:default and related telemetry#9119
rokonec merged 10 commits intodotnet:mainfrom
rokonec:rokonec/9063-skd-tl-defaults

Conversation

@rokonec
Copy link
Copy Markdown
Member

@rokonec rokonec commented Aug 10, 2023

Fixes part of #9063

Context

See #9063 for requirements.
This PR address changes in msbuild only. SDK changes will be in different PR. This PRs are independent and could be merged in any order.

Changes Made

  • introduce /tlp:default={true|false|auto} to signalize what is SDK intent of default behavior. Such default will achieve requirements of overriding default by user intent (arguments, env vars, rsp files)
  • introduced LoggingConfigurationTelemetry containing info about logger settings

Testing

  • locally
  • existing and new unit tests

Notes

Copy link
Copy Markdown

@donJoseLuis donJoseLuis left a comment

Choose a reason for hiding this comment

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

minor comments

Comment thread src/Build.UnitTests/TerminalLoggerConfiguration_Tests.cs
Comment thread src/Build.UnitTests/TerminalLoggerConfiguration_Tests.cs
Comment thread src/Build.UnitTests/TerminalLoggerConfiguration_Tests.cs
Comment thread src/Build/Logging/BinaryLogger/BinaryLogger.cs
Comment thread src/Build/Logging/FileLogger.cs
Comment thread src/MSBuild/XMake.cs Outdated
Comment thread src/Shared/UnitTests/MockLogger.cs
Copy link
Copy Markdown
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

Overall looks good.

I have couple annoying comments that you can aknowledge and skip. The SetConsoleMode needs to be addressed though - that would cause regressions

Comment thread src/Framework/Telemetry/LoggingConfigurationTelemetry.cs
Comment thread src/Framework/Telemetry/KnownTelemetry.cs Outdated
Comment thread src/Build/BackEnd/BuildManager/BuildManager.cs
Comment thread src/MSBuild/XMake.cs
Comment thread src/Shared/UnitTests/MockLogger.cs
Comment thread src/MSBuild/XMake.cs Outdated
@rokonec rokonec requested a review from JanKrivanek August 11, 2023 16:07
Copy link
Copy Markdown
Member

@JanKrivanek JanKrivanek left a comment

Choose a reason for hiding this comment

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

I like the final version. Looks good to go to me!

Comment thread src/Framework/Telemetry/LoggingConfigurationTelemetry.cs Outdated
Comment thread src/MSBuild/XMake.cs
Comment thread src/Build.UnitTests/TerminalLoggerConfiguration_Tests.cs
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.

5 participants