-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Disable useUserOverride in CultureContext #21990
Conversation
... never mind, apparently that constructor is not available in 😢 |
a028bda
to
9dda4f0
Compare
@@ -476,7 +476,7 @@ void M() | |||
} | |||
} | |||
"; | |||
using (new CultureContext("en-US")) | |||
using (new CultureContext(new CultureInfo("en-US", false))) |
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.
false [](start = 63, length = 5)
Consider using named argument. #Closed
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
LGTM |
Regex used: rg 'new CultureInfo' --files-with-matches | xargs sed -Ei 's/new CultureInfo\("("?[^"]*"?)"(, false)?\)/new CultureInfo("\1", useUserOverride: false)/' rg 'New CultureInfo' --files-with-matches | xargs sed -Ei 's/New CultureInfo\("("?[^"]*"?)"(, False)?\)/New CultureInfo("\1", useUserOverride:=False)/'
Pushed another commit that both takes into account @AlekseyTs's feedback to use named arguments, as well as doing a regex search for all |
retest windows_release_vs-integration_prtest please |
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
This is a test-only change.
CultureInfo has this documentation: The user might choose to override some of the values associated with the current culture of Windows through the regional and language options portion of Control Panel. For example, the user might choose to display the date in a different format or to use a currency other than the default for the culture. If the culture identifier associated with name matches the culture identifier of the current Windows culture, this constructor creates a CultureInfo object that uses those overrides, including user settings for the properties of the DateTimeFormatInfo instance returned by the DateTimeFormat property, and the properties of the NumberFormatInfo instance returned by the NumberFormat property.
This was causing one test on my machine to fail:
The fix is to explicitly override this behavior, by passing
useUserOverride = false
in a different constructor.Behavior testing in a console app:
produces
I don't know what team should review this, so I've just added roslyn-compiler. Feel free to add a
area-*
and the correct reviewer if there is a more appropriate team.