Review Request for project Threeten updates

[Xueming Shen]

(1) until(Temporal endExclusive, TEmporalUnit unit)
     -> Shouldn't we invoke requireNonNull() for unit before invoking unit.between(...)?

(2) It appears we started to use "endExclusive" in "until() methods, while the param
      has been renamed to be "exclusive" explicitly, personally I prefer still keep the
      word "exclusive" in its spec, or at least we want to be consistent, for example
      LocalDate.until(ChronoLocalDate) still keeps the "exclusive" after renaming.

(3) We have the erquireNonNull check pattern below in most places
     if (X instanceof Y) { ... }
     Objects.requireNonNull(X, "x");

     but the plus/minus(TemporalAmount amountToSubtract) appears to be

     Objects.requireNonoNull(...)
      if (X instanceof ...) {...}

(4) spec and impl of the until(end, unit) has been updated regarding passing the
     "converted" input temporal as the second argument for unit.between(...)

     If the unit is not a {@code ChronoUnit}, then the result of this method
     is obtained by invoking {@code TemporalUnit.between(Temporal, Temporal)}
     passing {@code this} as the first argument and the converted input temporal
     as the second argument.  

     I do see the TemporalUnit.between() has wording regarding the first and second must be
     the same "type/class", otherwise, it will send the ball back to Temporal,  then we might
     end up with a loop. My guess these new wording is to prevent a possible implementation
     deadlock? But it appears the super Temporal/ChronoLocalXYZ interfaces still claim
     the "input temporal as the second argument", though it also insists that

     "Note that the unit's {@code between} method must only be invoked if two temporal
     objects have exactly the same type evaluated by {@code getClass()}."

     and the sample shows it indeed passes the converted one. just a small typo here?

     (The ChronoLocalXYZImpl.until(end, unit) probably needs to update the api
     doc to explicitly describe this change as well, instead of "@Override", if the wording
     in Temporal/ChronoLocalXYz is indeed by design?)

-Sherman

On 10/01/2013 11:26 AM, roger riggs wrote:
> Please review these changes from the Threeten project for integration into jdk-tl.
>
> http://cr.openjdk.java.net/~rriggs/webrev-period-until-8023762-807-834-835/
>
> 8023762: Add ChronoPeriod interface and bind period to Chronology
> Summary: Make Period ISO-only, adding a Chronology-specific period concept
> Contributed-by: scolebourne at joda.org
>
> 8023763: Rename ChronoDateImpl
> Summary: Rename ChronoDateImpl to ChronoLocalDateImpl
> Contributed-by: scolebourne at joda.org
>
> 8023764: Optimize Period addition
> Summary: Optimise plus/minus for common cases
> Contributed-by: scolebourne at joda.org
>
> 8024835: Change until() to accept any compatible temporal
> Summary: Method until(Temporal,TemporalUnit) now uses from() to convert; Enhance from() methods where necessary
> Contributed-by: scolebourne at joda.org
>
> 8024807: Add getChronlogy() to CLDT/CZDT
> Summary: Alternative to method is clunky and hard to find
> Contributed-by: scolebourne at joda.org
>
> 8024834: Better return type for TemporalField resolve
> Summary: Allow resolve method to return more than just ChronoLocalDate
> Contributed-by: scolebourne at joda.org
>
> 8024999: Instant.Parse typo in example
> Summary: javadoc only fix to correct example to use "." and "Z"
>
> Thanks, Roger
>
>
>
>