Skip to content

Conversation

@jkotas
Copy link
Member

@jkotas jkotas commented Jul 31, 2021

@RalfKornmannEnvision noticed that ICU initialization is not getting trimmed when Invariant=true and PredefinedCulturesOnly is unspecified (https://gitter.im/dotnet/corert?at=610514ee36b0b97fa2d99aff). The problem is that the Setting class constructor that initializes ICU gets referenced by the PredefinedCulturesOnly property in this case. Moving the PredefinedCulturesOnly out of the Setting class fixes the problem.

@jkotas jkotas requested a review from tarekgh July 31, 2021 11:01
@ghost
Copy link

ghost commented Jul 31, 2021

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

Issue Details

@RalfKornmannEnvision have noticed that ICU initialization is not getting trimmed when Invariant=true and PredefinedCulturesOnly unspecified (https://gitter.im/dotnet/corert?at=610514ee36b0b97fa2d99aff). The problem is that the Setting class constructor that initializes ICU gets referenced by the PredefinedCulturesOnly property. Moving the PredefinedCulturesOnly out of the Setting class fixes the problem.

Author: jkotas
Assignees: -
Labels:

area-System.Globalization

Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Jul 31, 2021

@jkotas this changed made the test InvariantGlobalizationTrue to fail. It looks something used to get trimmed is not trimmed anymore.

CC @eerhardt

@jkotas jkotas merged commit a190d44 into dotnet:main Aug 1, 2021
@jkotas jkotas deleted the fix-icu branch August 1, 2021 19:18
static Settings()
{
if (!Invariant)
// Use GlobalizationMode.Invariant to allow ICU initialization to be trimmed when Invariant=false
Copy link
Member

Choose a reason for hiding this comment

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

when Invariant=false => when Invariant=true

@ghost ghost locked as resolved and limited conversation to collaborators Sep 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