On Fri, 11 Feb 2022 00:03:50 GMT, Naoto Sato <na...@openjdk.org> wrote:

>> Following the prior discussion [1], here is the PR for the subject 
>> enhancement. CSR has also been updated according to the suggestion.
>> 
>> [1] 
>> https://mail.openjdk.java.net/pipermail/core-libs-dev/2022-January/085175.html
>
> Naoto Sato has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Addressing review comments

Looks good.

src/java.base/share/classes/java/time/format/DateTimeFormatterBuilder.java line 
5103:

> 5101:          * @param timeStyle  the time style to use, may be null
> 5102:          */
> 5103:         LocalizedPrinterParser(FormatStyle dateStyle, FormatStyle 
> timeStyle) {

Can the constructors be `private`.  
The combination of package protected and the style of caller doing the 
validation makes me a bit nervous.
There should not be any callers outside of DateTimeFormatterBuilder.

src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java 
line 66:

> 64:      * Returns the formatting pattern for the requested template, 
> calendarType, and locale.
> 65:      * Concrete implementation of this method will retrieve
> 66:      * a java.time specific pattern from selected Locale Provider.

editorial: "from **the** selected Locale"...

src/java.base/share/classes/sun/text/spi/JavaTimeDateTimePatternProvider.java 
line 79:

> 77:     public String getJavaTimeDateTimePattern(String requestedTemplate, 
> String calType, Locale locale) {
> 78:         // default implementation throws exception
> 79:         throw new DateTimeException("Formatter is not available for the 
> requested template: " + requestedTemplate);

"Formatting **pattern** is not available"...

-------------

Marked as reviewed by rriggs (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7340

Reply via email to