Codereview request for 8003680: JSR 310: Date/Time API (original) (raw)

Alan Bateman Alan.Bateman at oracle.com
Wed Jan 16 12:08:01 UTC 2013

On 16/01/2013 00:13, Xueming Shen wrote:

Hi,

The Threeten project [1] is planned to be integrated into OpenJDK8 M6 milestone. Here is the webrev http://cr.openjdk.java.net/~sherman/8003680/webrev and the latest javadoc http://cr.openjdk.java.net/~sherman/8003680/javadoc Review comments can be sent to the threeten-dev email list [2] and/or core-libs-dev email list[3]. This is not a review of the API or implementation. Rather just a few random comments as I look through the webrev.

It looks to me that the zone rules compiler ends up in rt.jar, is that expected and is it actually used at runtime? On initial read I thought it was build-time only but I might be wrong. As per off-list discussion, it needs to run on the boot JDK to work in cross-compilation environments and so the dependency on java.time is an issue.

I see Formatter has been updated to support conversions of TemporalAccessor. Is the assert(width == -1) on L4067 right or was it copied from another print method? (Brian Burkhalter and I were chatting recently about an assert into another print method).

Also on Formatter then I think it would be good to put the tests in test/java/util/Formatter as someone touching the Formatter code might not know there are additional tests hiding down in the odd location test/java/time/test/java/util.

As you are adding a jdk_test target to test/Makefile then you will should also add the target to jprt.properties (btoh top-level repo and jdk/make). This is only interesting for Oracle build+test system of course.

Just on the tests, is the plan to push the TCK tests to the jdk as proposed in the webrev?

-Alan.

More information about the core-libs-dev mailing list