Re: [threeten-develop] [threeten-dev] Review request for changes to support Hijrah variants

[roger r. <rog...@or...>]

Thanks for the comments, updates below:

Webrev:   http://cr.openjdk.java.net/~rriggs/webrev-hijrah-variants/

Javadoc:  http://cr.openjdk.java.net/~rriggs/javadoc-hijrah-variants/


On 2/19/2013 5:35 AM, Stephen Colebourne wrote:
> HijrahChronology should have a blank line before
> <h3>Hijrah Calendar Variant Configuration</h3>
> Moreover, I assume that calendar.properties and its contents are JDK
> specific, not part of the spec. So, this is an "implementation note",
> not an "implementation spec"
Yes, I will change the subsection title.
>
> I note that other variants have been removed, so we only have umalqura now.
They will be restored when we have the data to support them.
>
> I think getCalendarType() needs to explain how the variant is handled,
> as effectively that method returns two pieces of information, or add a
> getCalendarVariant().
I will add a description.  CLDR only recently added the concept of a variant
and it needs to exist at the boundary where locale based tags are
mapped to Threeten semantics.  The alternative is to propagate the split
type and variant parameters into the 'of" method.

Having a single calendar type is more convenient but does raise the
issue of creating names from components and breaking apart again.
The "-" as a delimiter is ok since it is not permitted by CLDR/LDML.

>
> public ValueRange range(ChronoField field) is incorrect wrt retaining
> the BEFORE era
Please recheck, updated to include the full range of Year in the smallest
min and greatest max to allow for different calendars.
(Unless I misunderstand).
>
> I still think that the ids "Hirah", "islamic" and "islamicc" should
> map to umalqura.
CLDR proposed to deprecate "islamicc";  so that should not be included.

The chronology can be registered with multiple names; so all the lookups
return the Hijrah-umalqura chronology.

Thanks, Roger

>
> Stephen
>
>
> On 15 February 2013 21:16, roger riggs <rog...@or...> wrote:
>> Please review the changes to support multiple Islamic calendar variants:
>>   - Only the Umm alQura variant is supported so far and is supported by
>> limited
>>     placeholder data until the full data can be acquired and validated.
>>   - Tests have been updated to work with the updated API and available data
>>   - The use of Locale 'ca' and 'cv' values has been added to
>> Chronology.ofLocale
>>   - Properties are added to lib/calendars.properties to identify the
>> variants
>>     by calendar type and to identify the resource containing the calendar
>> data.
>>
>> Webrev:   http://cr.openjdk.java.net/~rriggs/webrev-hijrah-variants/
>>
>> Javadoc:  http://cr.openjdk.java.net/~rriggs/javadoc-hijrah-variants/
>>
>> Note: there are several related issues to be addressed after this change:
>>   - The API for Era.date(xxx) does not lend itself to support of calendars
>>     with multiple variants; Eras do not have any knowledge of a specific
>> chronology
>>   - Performance of initialization at startup
>>   - Enumerations of Eras
>>
>> --
>> Thanks, Roger
>>
>> Oracle Java Platform Group
>>
>> Oracle is committed to developing practices and products that help protect
>> the environment
>>
>>
>>
>> ------------------------------------------------------------------------------
>> The Go Parallel Website, sponsored by Intel - in partnership with Geeknet,
>> is your hub for all things parallel software development, from weekly
>> thought
>> leadership blogs to news, videos, case studies, tutorials, tech docs,
>> whitepapers, evaluation guides, and opinion stories. Check out the most
>> recent posts - join the conversation now.
>> http://goparallel.sourceforge.net/
>> _______________________________________________
>> threeten-develop mailing list
>> thr...@li...
>> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>>