Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi Nadeesh, Looks fine, Thanks, Roger On 1/27/2016 11:34 AM, nadeesh tv wrote: Hi all, Thanks for the suggestions. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.01/ Regards, Nadeesh TV On 1/25/2016 10:24 PM, Roger Riggs wrote: Hi Stephen, Nadeesh,

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

On 29/01/16 14:52, Roger Riggs wrote: Hi Nadeesh, Looks fine, Thanks, Roger On 1/27/2016 11:34 AM, nadeesh tv wrote: Hi all, Thanks for the suggestions. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.01/ +1 This looks fine. Martin, Doug, I assume you

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

I missed that this was modifying jsr166 files - looking now... On Fri, Jan 29, 2016 at 7:12 AM, Chris Hegarty wrote: > On 29/01/16 14:52, Roger Riggs wrote: >> >> Hi Nadeesh, >> >> Looks fine, >> >> Thanks, Roger >> >> >> On 1/27/2016 11:34 AM, nadeesh tv wrote: >>> >>>

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

I propose that we the jsr166 maintainers take over this change (sorry for butting in!), pushing it into openjdk from jsr166 CVS. The tests as they are written today won't work because TimeUnit/Basic.java is not a testng test. But I don't think we should fix that - instead, tests for these methods

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

In the ideal fix, all methods that take the combination (long, TimeUnit) would be supplemented by an override that takes (Duration). eg. Future.get(long, TimeUnit) has an additional get(Duration). However, this is a lot of work, might be unpopular and would be slower for j.u.concurrent use. The

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Here is a proposed alternative patch (against jsr166 CVS). Code is reworded and reformatted. Tests are junit-ified. Round-trip tests are added. toChronoUnit no longer throws IAE, because it cannot (and we commit to having ChronoUnit be a superset of TimeUnit, in perpetuity; toChronoUnit will

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

On Fri, Jan 29, 2016 at 11:17 AM, Roger Riggs wrote: > Hi Martin, > > Where did IllegalMonitorStateException creep in from? Oops. My Emacs keeps suggesting the wrong completion. Fixed. > Should I update the CCC I started or will you start fresh? If you could update

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi Martin, Where did IllegalMonitorStateException creep in from? Should I update the CCC I started or will you start fresh? Otherwise, ok. Roger On 1/29/2016 2:12 PM, Martin Buchholz wrote: Here is a proposed alternative patch (against jsr166 CVS). Code is reworded and reformatted. Tests

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi Martin, On 1/29/2016 2:28 PM, Martin Buchholz wrote: On Fri, Jan 29, 2016 at 11:17 AM, Roger Riggs wrote: Hi Martin, Where did IllegalMonitorStateException creep in from? Oops. My Emacs keeps suggesting the wrong completion. Fixed. Should I update the CCC I

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

On Fri, Jan 29, 2016 at 9:37 AM, Stephen Colebourne wrote: > In the ideal fix, all methods that take the combination (long, > TimeUnit) would be supplemented by an override that takes (Duration). > eg. Future.get(long, TimeUnit) has an additional get(Duration). > However,

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi all, Thanks for the suggestions. Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.01/ Regards, Nadeesh TV On 1/25/2016 10:24 PM, Roger Riggs wrote: Hi Stephen, Nadeesh, TimeUnit.toChronoUnit is a static method. It seems redundant to have to pass an instance

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi all, Please see the updated webrev http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV On 1/25/2016 9:01 PM, Stephen Colebourne wrote: Typo "TimeUnitequivalent" Otherwise looks good. thanks Stephen On 25 January 2016 at 15:25, nadeesh tv

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi Stephen, Nadeesh, TimeUnit.toChronoUnit is a static method. It seems redundant to have to pass an instance to a static method of its type. cu = TimeUnit.toChronoUnit(TimeUnit.SECONDS); Instead of: TimeUnit tu = TimeUnit.SECONDS; ChronoUnit cu = tu.toChronoUnit(); Minor

RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Hi all, Please review a fix for conversion between Chronounit and Timeunit Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 webrev: http://cr.openjdk.java.net/~ntv/8141452/webrev.00/ -- Thanks and Regards, Nadeesh TV

Re: RFR:JDK-8141452:Convert between TimeUnit and ChronoUnit

Typo "TimeUnitequivalent" Otherwise looks good. thanks Stephen On 25 January 2016 at 15:25, nadeesh tv wrote: > Hi all, > > Please review a fix for conversion between Chronounit and Timeunit > > Bug ID : https://bugs.openjdk.java.net/browse/JDK-8141452 > > webrev: