On 01/16/2013 04:08 AM, Alan Bateman wrote:
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.
Yes, looking into rewriting or moving some pieces around to satisfy the
cross-compilation env.
No, those should not be in rt.jar, at least for now.
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).
It's copy/pasted from print(StringBuffer, Calendar...)
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.
Yes, understood. The concern is that, at least for now, it is more likely we are
going to change the java/time and we forget there is another test case at
java/util/Formatter:-) Maybe it is more convenient to keep it here for a little
while until the threeten is stable enough. The "location" is odd, mainly to
satisfy the TestNG requirement (for tests with their own package). The test
is written to use TestNG.
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.
Sure, will added.
Just on the tests, is the plan to push the TCK tests to the jdk as proposed in
the webrev?
Those tck tests also serve as unit tests. They are so named for the convenience
that TCK guys can just pick them up and drop into their suite easily.
-Sherman
