Hi Naoto,
Thanks for the explanation. I agree on the needs for maintaining
backward compatibility. The patch looks good me then.
Best,
Joe
On 5/20/2020 4:49 PM, naoto.s...@oracle.com wrote:
Hi Joe, thanks for the review.
On 5/20/20 4:10 PM, Joe Wang wrote:
Hi Naoto,
I don't seem to see the DateFormat class or the getDateInstance
methods specify how errors may be handled (or logged). Is that stated
somewhere else?
Description of the system property "java.locale.providers" is in the
class description of java.util.spi.LocaleServiceProvider class, in
which the possible provider names can only be
CLDR/COMPAT/SPI/HOST/JRE. So possible error with this system property
is user's typo of the provider names. Although the behavior is not
described, wrong names have been simply ignored at runtime. The
problem here is that user cannot tell that he's done typo, and this
fix is exactly to address it.
In other cases, I see that you've changed it to throw
ServiceConfigurationError, that looks to me may be better than a log
as a configuration error (or specifying wrong provider) sounds to me
more severe than info.
Those exceptions will never happen in normal situation, since those
locations are loading SPI/HostLocaleProviderAdapter class(es) that are
the JDK classes (classes/methods are known to exist).
On the other hand, I kept exception handlers in LocaleProviderAdapter
to generate Level.INFO log, as this is for the user's typo of the
provider names (explained above). Again this should be ignored and
replaced/continue with proper default behavior, and letting user know
the typo. If this were throwing SCError, it would break compatibility.
Of the three HostLocaleProviderAdapterImpl, the one for unix is
deleted, is there a specific reason?
Because it is simply not necessary (empty class). It was meant to be
implemented later, but the host provider on Unixes has never been
requested. Probably because OS date/time/number settings in Unixes are
less important to Java clients.
HTH,
Naoto
Best,
Joe
On 5/20/2020 10:29 AM, naoto.s...@oracle.com wrote:
Hello,
Please review the fix to the following issue:
https://bugs.openjdk.java.net/browse/JDK-8245241
The proposed changeset is located at:
https://cr.openjdk.java.net/~naoto/8245241/webrev.00/
Incorrect user-provided provider preference is supposed to be
logged. However it is not so because it is using
PlatformLogger(SurrogateLogger) which uses the default logging
level. I changed it to use j.l.System's logger and bumped it to INFO
level (it should notify the user by default). By taking this
opportunity, I did some clean-up in locale provider adapter's
logging related code as well.
Naoto
