CLDR-17313 st: only build supplemental factory once #3454
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
the supplemental disk factory should be a singleton. also. use the Suppliers.memoize() idiom in more places.
macchiati
approved these changes
Jan 17, 2024
btangmu
reviewed
Jan 18, 2024
| () -> { | ||
| final File[] list = getFileBases(); | ||
| CLDRConfig config = CLDRConfig.getInstance(); | ||
| // may fail at server startup time- should do this through setup mode |
There was a problem hiding this comment.
"may fail at server startup time" -- if so, what will happen?
There was a problem hiding this comment.
It'll actually explain to the user that the survey tool isn't configured yet and invite hem to Edit cldr.properties.
There was a problem hiding this comment.
so it would only be expected to fail if cldr.properties isn't properly configured yet? if so, the comment could be less alarming if it spelled that out...
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.
the supplemental disk factory should be a singleton. also. use the Suppliers.memoize() idiom in more places.
Because it's a singleton,
getSupplementalDiskFactory()is now cheap to call.Note: ✅ Sanity tested locally.
CLDR-17313
ALLOW_MANY_COMMITS=true