Hi Stephen, The webrev has been updated accordingly.
http://cr.openjdk.java.net/~sherman/8033662/webrev/ Thanks! -Sherman On 03/24/2014 08:59 AM, Stephen Colebourne wrote:
I don't think think email was delivered to me. (And I think another from you also wasn't). The email is in the core-libs-dev archive though. I think the approach you changed it to is fine in principal although whether the tidy up should be in jdk8u is a different matter. In your changes I think that toParsed() should be renamed to toUnresolved(). And there still seem to be instance variables for effectiveZone and effectiveChrono (which I thought you were trying to remove). thanks Stephen On 24 March 2014 15:00, Xueming Shen <xueming.s...@oracle.com <mailto:xueming.s...@oracle.com>> wrote: Stephen, I commented on this one last week. I did not see the response, is it something worth considering? or you believe the original approach is the right way to go. Thanks! -Sherman -------- Original Message -------- Message-ID: <532b6f87.3000...@oracle.com> <mailto:532b6f87.3000...@oracle.com> Date: Thu, 20 Mar 2014 15:45:27 -0700 From: Xueming Shen <xueming.s...@oracle.com> <mailto:xueming.s...@oracle.com> User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.17) Gecko/20110414 Thunderbird/3.1.10 MIME-Version: 1.0 To: core-libs-dev@openjdk.java.net <mailto:core-libs-dev@openjdk.java.net> Subject: Re: RFR: JDK-8033662 DateTimeFormatter parsing ignores withZone() References: <CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.com> <mailto:CACzrW9BTWhehV+oC-aEQAEkLtTA4XO6z-F5=jevyv-d35+y...@mail.gmail.com> <CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com> <mailto:CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com> In-Reply-To: <CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com> <mailto:CACzrW9BcmmesdKQBrtgrAc=12hn3uowjvwzxskmfcksajea...@mail.gmail.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Hi Stephen, Given the fact that the parser context is no longer public, the parsed from the formatter is either unresolved or resolved, just wonder if we really need those "effective" chrono and zone fields in Parsed. It appears perfect for me to simply keep these info inside the parser context and return the unresolved and resolved based on the formatter's request, for example http://cr.openjdk.java.net/~sherman/8033662/webrev2/ <http://cr.openjdk.java.net/%7Esherman/8033662/webrev2/> -Sherman On 03/20/2014 11:24 AM, Stephen Colebourne wrote: > Hi there, > It would be great if I could get a review please. > > The patch is viewable in plain text at JIRA (for IP reasons): > https://bugs.openjdk.java.net/secure/attachment/19216/ParseWithZone.patch > > The same patch is viewable in a nice format at GitHub > https://gist.github.com/jodastephen/9505761 > > This really needs to make 8u20 IMO, so I need to get it into jdk9 first > thanks > Stephen > > > > On 12 March 2014 12:29, Stephen Colebourne<scolebou...@joda.org> <mailto:scolebou...@joda.org> wrote: >> This is a request for review of this bug: >> https://bugs.openjdk.java.net/browse/JDK-8033662 >> and the duplicate: >> https://bugs.openjdk.java.net/browse/JDK-8033659 >> >> The javadoc of the method java.time.format.DateTimeFormatter::withZone says: >> "If no zone has been parsed, then this override zone will be included >> in the result of the parse where it can be used to build instants and >> date-times." >> However, the implementation doesn't obey this. >> >> This is a very unfortunate bug that makes some date-time parsing a lot harder. >> >> A second related bug in an egde case - not properly handling a >> ChronoZonedDateTime from TemporalField.resolve - is also tested for >> and fixed. >> >> >> Proposed patch: >> https://gist.github.com/jodastephen/9505761 >> The patch includes no spec changes. >> The patch includes new and refactored TCK tests. The new tests for >> withZone() and withChronology() are based on the spec. >> >> I need a reviewer and a committer please. >> thanks
