+1
On 3/9/20 12:05 PM, Joe Wang wrote:
The changes look good to me.
Best,
Joe
On 3/9/20 1:44 AM, Stephen Colebourne wrote:
Fine by me, but I'm not an OpenJDK Reviewer
Stephen
On Mon, 9 Mar 2020 at 03:05, wrote:
Thanks, Stephen.
Updated the webrev for those two suggestions:
The changes look good to me.
Best,
Joe
On 3/9/20 1:44 AM, Stephen Colebourne wrote:
Fine by me, but I'm not an OpenJDK Reviewer
Stephen
On Mon, 9 Mar 2020 at 03:05, wrote:
Thanks, Stephen.
Updated the webrev for those two suggestions:
http://cr.openjdk.java.net/~naoto/8239836/webrev.04/
Fine by me, but I'm not an OpenJDK Reviewer
Stephen
On Mon, 9 Mar 2020 at 03:05, wrote:
>
> Thanks, Stephen.
>
> Updated the webrev for those two suggestions:
>
> http://cr.openjdk.java.net/~naoto/8239836/webrev.04/
>
> Naoto
>
> On 3/8/20 4:22 PM, Stephen Colebourne wrote:
> >
Thanks, Stephen.
Updated the webrev for those two suggestions:
http://cr.openjdk.java.net/~naoto/8239836/webrev.04/
Naoto
On 3/8/20 4:22 PM, Stephen Colebourne wrote:
standardOffsets[0] == wallOffsets[0] needs to use .equals()
Unrelated, there is a Javadoc/spec error at line 578 of the
standardOffsets[0] == wallOffsets[0] needs to use .equals()
Unrelated, there is a Javadoc/spec error at line 578 of the "new"
file. It should be getValidOffsets, not getOffset.
Apart from that, it looks good.
thanks
Stephen
On Wed, 4 Mar 2020 at 19:06, wrote:
>
> Hi Stephen,
>
> Thank you
Hi Stephen,
Thank you for the detailed explanation and correcting the
implementation. I incorporated all the suggestions below, as well as
adding a new test for isFixedOffset() implementation (with some clean-up):
https://cr.openjdk.java.net/~naoto/8239836/webrev.03/
Please take a look at
I fear more changes are needed here.
1) The code is isFixedOffset() is indeed wrong, but the fix is
insufficient. The zone is fixed if baseStandardOffset=baseWallOffset
and all three other lists are empty. A fixed offset rule is the
equivalent of a ZoneOffset. See ZoneId.normalized() for the code
Looks good to me.
Thanks,
Joe
On 3/3/20 10:15 AM, naoto.s...@oracle.com wrote:
Thanks, Joe. Updated:
http://cr.openjdk.java.net/~naoto/8239836/webrev.02/
Naoto
On 3/3/20 8:59 AM, Joe Wang wrote:
On 3/3/20 8:27 AM, naoto.s...@oracle.com wrote:
Hi Joe, thanks for the review.
Are you
Thanks, Joe. Updated:
http://cr.openjdk.java.net/~naoto/8239836/webrev.02/
Naoto
On 3/3/20 8:59 AM, Joe Wang wrote:
On 3/3/20 8:27 AM, naoto.s...@oracle.com wrote:
Hi Joe, thanks for the review.
Are you suggesting something like
isFixedOffset() {
return isFixedOffset;
}
Yes,
On 3/3/20 8:27 AM, naoto.s...@oracle.com wrote:
Hi Joe, thanks for the review.
Are you suggesting something like
isFixedOffset() {
return isFixedOffset;
}
Yes, something like that. It could be initiated while the rules is
constructed. I feel it might be semantically clearer as
Hi Joe, thanks for the review.
Are you suggesting something like
isFixedOffset() {
return isFixedOffset;
}
Naoto
On 3/2/20 11:20 PM, Joe Wang wrote:
Hi Naoto,
The fix would be fine if you want to keep it as is since it does work.
I noticed though that for standard zones (the ones
Hi Naoto,
The fix would be fine if you want to keep it as is since it does work.
I noticed though that for standard zones (the ones loaded from tz
database), savingsInstantTransitions and standardTransitions are
consistent in that they are both empty for the standard zones, e.g.
Etc/GMT, and
Looks good.
Give it a day to see if anyone else has comments.
Thanks, Roger
On 3/2/20 1:35 PM, naoto.s...@oracle.com wrote:
Hi Roger, thanks for the review.
On 3/2/20 8:44 AM, Roger Riggs wrote:
Hi Naoto,
look ok.
ZoneRules.java: 488, 644, 761, 791
I'd suggest calling isFixedOffset()
Hi Roger, thanks for the review.
On 3/2/20 8:44 AM, Roger Riggs wrote:
Hi Naoto,
look ok.
ZoneRules.java: 488, 644, 761, 791
I'd suggest calling isFixedOffset() instead of repeating the code:
standardTransitions.length == 0 && savingsInstantTransitions.length == 0
Modified as suggested:
Hi Naoto,
look ok.
ZoneRules.java: 488, 644, 761, 791
I'd suggest calling isFixedOffset() instead of repeating the code:
standardTransitions.length == 0 && savingsInstantTransitions.length == 0
It should be inlined in cases where the performance matters.
Thanks, Roger
On 2/27/20 3:41 PM,
Hello,
Please review the fix to the following issue:
https://bugs.openjdk.java.net/browse/JDK-8239836
The proposed changeset is located at:
https://cr.openjdk.java.net/~naoto/8239836/webrev.00/
It turned out that 'transitionList' is not necessarily a superset of
16 matches
Mail list logo
