-
Couldn't load subscription status.
- Fork 5.2k
Add missing CultureInfo.InvariantCulture use to LogValuesFormatter #117008
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
Conversation
|
Tagging subscribers to this area: @dotnet/area-extensions-logging |
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.
Pull Request Overview
This PR ensures that LogValuesFormatter uses CultureInfo.InvariantCulture when formatting values and adds a corresponding test to validate culture-invariant output.
LogValuesFormatterformatting methods now explicitly passCultureInfo.InvariantCulturefor placeholder indices and enumerable elements.- A new unit test
FormatMessage_UsesInvariantCulturehas been added toLoggerExtensionsTest.csto verify behavior under a non-default culture. - Imported
System.Globalizationwhere needed.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs | Use CultureInfo.InvariantCulture in ToString calls for consistent output |
| src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerExtensionsTest.cs | Added test FormatMessage_UsesInvariantCulture and imported System.Globalization |
Comments suppressed due to low confidence (1)
src/libraries/Microsoft.Extensions.Logging.Abstractions/src/LogValuesFormatter.cs:69
- Ensure the
System.Globalizationnamespace is imported at the top of this file so thatCultureInfo.InvariantCultureresolves correctly and the code compiles without errors.
vsb.Append(_valueNames.Count.ToString(CultureInfo.InvariantCulture));
src/libraries/Microsoft.Extensions.Logging/tests/Common/LoggerExtensionsTest.cs
Outdated
Show resolved
Hide resolved
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.
LGTM, Thanks!
Fixes #116979