Use NumberFormatter to format metric values #14017
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sgiehl
added
not-in-changelog
|
In a quick test German continues to use |
|
Money values are now formatted as |
|
kind of expected, as the formatter now uses the formats defined by CLDR. That means in some languages there will now be a space between the currency and the number in others not. Also the currency symbol is pre- or appended depending on the language. |
sgiehl
force-pushed
the
metricformat
branch
3 times, most recently
from
70a850f
to
a4084e2
Compare
|
updated the branch and all test files. should be ready for a final review and merge... |
diosmosis
reviewed
May 13, 2019
plugins/PrivacyManager/tests/System/expected/test_allSites__Live.getLastVisitsDetails_year.xml
Show resolved
Hide resolved
|
Saw one possible issue, everything else looks good. |
|
@diosmosis that was actually a general problem with the numberformatter. Should be fixed in 8a6c950 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This will change all metric formatting's to use the local formats of NumberFormatter.
Not yet sure if that will have any side effects besides the change of all exports.
fixes #13994