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

[Stephen Colebourne]

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"

I note that other variants have been removed, so we only have umalqura now.

I think getCalendarType() needs to explain how the variant is handled,
as effectively that method returns two pieces of information, or add a
getCalendarVariant().

public ValueRange range(ChronoField field) is incorrect wrt retaining
the BEFORE era

I still think that the ids "Hirah", "islamic" and "islamicc" should
map to umalqura.

Stephen


On 15 February 2013 21:16, roger riggs <roger.riggs at oracle.com> 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
> threeten-develop at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/threeten-develop
>