Skip to content
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

Make GlobalizationMode code consistent #55039

Merged
merged 2 commits into from
Jul 1, 2021

Conversation

tarekgh
Copy link
Member

@tarekgh tarekgh commented Jul 1, 2021

Address the comment #54247 (comment) in the PR #54247

@tarekgh
Copy link
Member Author

tarekgh commented Jul 1, 2021

CC @marek-safar

@ghost
Copy link

ghost commented Jul 1, 2021

Tagging subscribers to this area: @tarekgh, @safern
See info in area-owners.md if you want to be subscribed.

Issue Details

Address the comment #54247 (comment) in the PR #54247

Author: tarekgh
Assignees: -
Labels:

area-System.Globalization

Milestone: -

…GlobalizationMode.cs

Co-authored-by: Stephen Toub <stoub@microsoft.com>
Copy link
Member

@eerhardt eerhardt left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the follow up.

@tarekgh tarekgh merged commit 9c060fe into dotnet:main Jul 1, 2021
@tarekgh tarekgh deleted the MakeGlobalizationModeConsistent branch July 1, 2021 23:32
Copy link
Member

@safern safern left a comment

Choose a reason for hiding this comment

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

LGTM

Comment on lines +14 to +15
internal static bool Invariant { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.Invariant", "DOTNET_SYSTEM_GLOBALIZATION_INVARIANT");
internal static bool PredefinedCulturesOnly { get; } = AppContextConfigHelper.GetBooleanConfig("System.Globalization.PredefinedCulturesOnly", "DOTNET_SYSTEM_GLOBALIZATION_PREDEFINED_CULTURES_ONLY", Invariant);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think using read-only fields would be more appropriate for a private class

thaystg added a commit to thaystg/runtime that referenced this pull request Jul 2, 2021
* origin/main: (27 commits)
  [mono][llvm] Only emit 'LLVM failed' messages on verbosity > 0. (dotnet#55060)
  Http2Stream throws a wrapped Http2ConnectionException on GO_AWAY (dotnet#54625)
  [main] Update dependencies from dnceng/internal/dotnet-optimization dotnet/arcade dotnet/xharness dotnet/hotreload-utils (dotnet#55007)
  disable a failing test. (dotnet#55063)
  [mono][wasm] Disable some tests which crash on AOT. (dotnet#55054)
  Fix fix_allocation_context for regions (dotnet#54931)
  Delete stale references to System.IO.FileSystem.Primitives (dotnet#55041)
  Add binplaced analyzers to ASP.NET transport package (dotnet#55042)
  [mono] Enable many HardwareIntrinsic tests on wasm
  Delete `compQuirkForPPP`. (dotnet#55050)
  [Mono] Condition Workload AOT import to be osx only (dotnet#55040)
  package native quic library (dotnet#54992)
  Make GlobalizationMode code consistent (dotnet#55039)
  Expand PerfMap format to support metadata for symbol indexation (dotnet#53792)
  [debugger]Componentize debugger (dotnet#54887)
  [Mono] Include loaded interpreter methods as EventPipe session rundown method events. (dotnet#54953)
  Delete stale ActiveIssue from HttpHeadersTest (dotnet#55027)
  Poison address-exposed user variables in debug (dotnet#54685)
  Recategorize emsdk dependency (dotnet#55028)
  Remove the the wasm AOT specific test project exclusions (dotnet#54988)
  ...
@ghost ghost locked as resolved and limited conversation to collaborators Aug 1, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants