RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec

2014-02-28 Thread roger riggs

Please review this bug fix from Steve Colebourne for java.time parsing of
fixed width adjacent values. The patch includes new tests.

Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/

Thanks, Roger



Re: RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec

2014-02-28 Thread Lance Andersen - Oracle
Looks Ok.  Kind of surprised the tck tests have no assertion details in the 
tests.  Minor nit would have been nice to have even a minor comment for the new 
method DateTimeFormatterBuilder though that seems to be the norm in some 
scenarios for the smaller methods.


On Feb 28, 2014, at 4:48 PM, roger riggs wrote:

 Please review this bug fix from Steve Colebourne for java.time parsing of
 fixed width adjacent values. The patch includes new tests.
 
 Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/
 
 Thanks, Roger
 


Lance Andersen| Principal Member of Technical Staff | +1.781.442.2037
Oracle Java Engineering 
1 Network Drive 
Burlington, MA 01803
lance.ander...@oracle.com



Re: RFR 9: 8032491: DateTimeFormatter fixed width adjacent value parsing does not match spec

2014-02-28 Thread roger riggs

Hi Lance,

I'll see if I can add some comments.

The style for tests in java.time is less focused on statements in the 
spec due to
the way the API developed. Perhaps the tests should have been just 
regular unit tests.


Thanks, Roger


On 2/28/2014 5:05 PM, Lance Andersen - Oracle wrote:

Looks Ok.  Kind of surprised the tck tests have no assertion details in the 
tests.  Minor nit would have been nice to have even a minor comment for the new 
method DateTimeFormatterBuilder though that seems to be the norm in some 
scenarios for the smaller methods.


On Feb 28, 2014, at 4:48 PM, roger riggs wrote:


Please review this bug fix from Steve Colebourne for java.time parsing of
fixed width adjacent values. The patch includes new tests.

Webrev:
  http://cr.openjdk.java.net/~rriggs/webrev-parse-adjacent-8032491/

Thanks, Roger