I think thats ready to be merged, thanks Stephen
On 30 November 2015 at 11:26, nadeesh tv <nadeesh...@oracle.com> wrote: > Hi all, > Please see the updated webrev > http://cr.openjdk.java.net/~ntv/8142936/webrev.02/ > Regards, > Nadeesh TV > > On 11/27/2015 11:20 PM, Stephen Colebourne wrote: >> >> "Overall, looks pretty good. >> >> There a a number of double spaces that should be single spaces in the >> Javadoc. >> >> Change >> "This is based on the standard definition of a day has 24 hours." >> to >> "This is based on the standard definition of a day as 24 hours." >> ("has" to "as") >> There are three places to fix this. >> >> The toMillisPart() docs could do with some rework. >> change >> "number of milliseconds part in the nanosecond part of the duration" >> to >> "number of milliseconds part of the duration" >> >> try this: >> "This returns the milliseconds part by dividing the number of >> nanoseconds by 1,000,000." >> >> On the tests. >> >> There is no test for toSeconds(). >> >> For the other tests, I have been bitten before by not testing edge cases. >> A test for zero, and for a negative round number would be good. >> eg. for toHoursPart() the round number would be a duration of exactly >> minus 2 hours. >> >> Duration.ofHours(2).toDaysPart() = 0 >> Duration.ofHours(2).toHoursPart() = 2 >> Duration.ofHours(2).toMinutesPart() = 0 >> Duration.ofHours(2).toSecondsPart() = 0 >> Duration.ofHours(2).toMillisPart() = 0 >> Duration.ofHours(2).toNanosPart() = 0 >> >> thus really six tests are needed each time. The best way to achieve >> this is using @DataProvider in TestNG, where you can setup a data grid >> of inputs and 6 expected outputs. >> >> thanks >> Stephen >> >> >> >> >> On 26 November 2015 at 06:41, nadeesh tv <nadeesh...@oracle.com> wrote: >>> >>> Hi all, >>> >>> Please review a fix for >>> >>> Bug Id - https://bugs.openjdk.java.net/browse/JDK-8142936 >>> >>> -Enhance Duration by adding toNanosPart() , >>> toMillisPart(),toSecondsPart(),toMinutesPart(),toHoursPart(),toDaysPart() >>> methods . >>> - Had to rename private BigDecimal toSeconds() -> private >>> BigDecimal >>> toBigDecimalSeconds() >>> >>> >>> webrev - http://cr.openjdk.java.net/~ntv/8142936/webrev.01/ >>> >>> -- >>> Thanks and Regards, >>> Nadeesh TV >>> <div id="DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2"><table style="border-top: >>> 1px solid #aaabb6; margin-top: 10px;"> >> >> <tr> >> <td style="width: 105px; padding-top: 15px;"> >> <a >> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"; >> target="_blank"><img >> src="https://ipmcdn.avast.com/images/logo-avast-v1.png"; style="width: >> 90px; height:33px;"/></a> >> </td> >> <td style="width: 470px; padding-top: 20px; color: >> #41424e; >> font-size: 13px; font-family: Arial, Helvetica, sans-serif; >> line-height: 18px;">This email has been sent from a virus-free >> computer protected by Avast. <br /><a >> >> href="https://www.avast.com/?utm_medium=email&utm_source=link&utm_campaign=sig-email&utm_content=webmail"; >> target="_blank" style="color: #4453ea;">www.avast.com</a> >> </td> >> </tr> >> </table><a href="#DDB4FAA8-2DD7-40BB-A1B8-4E2AA1F9FDF2" width="1" >> height="1"></a></div> > > > -- > Thanks and Regards, > Nadeesh TV >
![https://ipmcdn.avast.com/images/logo-avast-v1.png"](https://ipmcdn.avast.com/images/logo-avast-v1.png"){kind=link}