Skip to content

Obsolete GlobalConfiguration #616

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

Merged
merged 8 commits into from
Feb 28, 2019
Merged

Conversation

angularsen
Copy link
Owner

@angularsen angularsen commented Feb 25, 2019

Fixes #600 .

So it can be more easily removed in v5.

  • Mark GlobalConfiguration as obsolete
  • Use Thread.CurrentThread.CurrentUICulture as backing value ⚠️
  • Update usages to use Thread.CurrentThread.CurrentUICulture directly

This is kind of a breaking change. Before it would initialize to CurrentUICulture upon first static access to GlobalConfiguration and that value would no longer match per-thread - but rather be globally static across threads. With this PR, it would instead match .NET's per-thread setup - which one could argue is a bug or a feature that we didn't - but since GlobalConfiguration was introduced in v4 in December and I don't think many/anyone relies on this initialize once behavior, I vote to take this in now.

@tmilnthorp
Copy link
Collaborator

I think we should probably be using CurrentCulture, not CurrentUICulture. At least for the number formatting. Maybe CurrentUICulture is more appropriate for the abbreviations?

I found an interesting quote here:

Often times they are both the same. But on my system they would be different: I prefer my numbers and dates in the German format, so the CurrentCulture would be German, but I also prefer all my applications in English, so the CurrentUICulture would be English.

@angularsen
Copy link
Owner Author

Yes, that matches how I remember it when reading up on that a long time ago. CurrentCulture for numbers, CurrentUICulture for abbreviations and other texts tied to language/culture.
However, for this PR, we can't do that change I think. Changing backing value to current thread's culture is fine I think, should not affect many - only those actively manipulating this value from multiple threads.
Changing it from CurrentUICulture to CurrentCulture sounds like would affect a lot more people, such as that german guy you mentioned.

I'm happy to address this in v5, but for this PR I think it's out of scope?

@tmilnthorp
Copy link
Collaborator

Agreed!

@angularsen angularsen merged commit c4cab69 into master Feb 28, 2019
@angularsen angularsen deleted the obsolete-globalconfiguration branch February 28, 2019 21:19
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.

2 participants