Hi Naoto,
thanks for reviewing. Comments below.
On 29/08/2012 19:55, Naoto Sato wrote:
Hi Sean,
It looks good overall. There are some minor comments that I have
below. All in Currency.java
Line 666: Comments refer to "GMT", while the class description uses
"UTC." I'd use "UTC" here as well (and in the actual code on line 728).
Agreed. I've only used "GMT" now in the datestamp string examples. I
think that's the normal case ?
Line 681, "countOccurrences(curdata, ',') == 3": Should "==" be ">="?
True - good to catch/flag any format issue.
Line 727: For locale invariant operation, I'd use "Locale.ROOT".
Done.
Line 731-734: These can be simplified to "return
System.currentTimeMillis() >= time;"
Done. much cleaner!
new webrev : http://cr.openjdk.java.net/~coffeys/webrev.7180362.2.jdk8/
<http://cr.openjdk.java.net/%7Ecoffeys/webrev.7180362.2.jdk8/>
regards,
Sean.
Naoto
On 8/28/12 6:48 AM, Seán Coffey wrote:
7180362 deals with an enhancement to allow the JRE specify cutover dates
when currency.properties file is provided. I've added the required
syntax to the new javadoc comments in Currency class.
bug report :http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7180362
webrev : http://cr.openjdk.java.net/~coffeys/webrev.7180362.1.jdk8/
<http://cr.openjdk.java.net/%7Ecoffeys/webrev.7180362.1.jdk8/>
I hope to port an almost identical change to 7u shortly. (minus API
javadoc comments)
I've kept the testcase logic different to the JRE logic around how the
new property is parsed to better
test the golden result expectations. (both approaches should be equal)
Regards,
Sean.
