Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Duration converters #86

Merged

Conversation

johanandren
Copy link
Contributor

Refs #85

else FiniteDuration(duration.getSeconds, TimeUnit.SECONDS)
} else {
FiniteDuration(
duration.getSeconds * 1000000000 + duration.getNano,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should probably be a Math.multiplyExact here though and an overflow check after adding the nano part.

@jbgi
Copy link

jbgi commented Apr 24, 2018

any update on this one? @johanandren maybe it could be integrated into https://github.com/johanandren/timeforscala if not here?

@SethTisue
Copy link
Member

not sure why/how this fell through the cracks...

@johanandren could you update this to add a section to README.md? then we can merge, I think

* @throws IllegalArgumentException If the given Java Duration is out of bounds of what can be expressed with the
* Scala Durations.
*/
final def toScala(duration: java.time.Duration): scala.concurrent.duration.Duration = {
Copy link

@jbgi jbgi Apr 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this would be more useful if it returns a FiniteDuration.

Copy link

@jbgi jbgi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think toScala should return FiniteDuration.

@johanandren
Copy link
Contributor Author

I'll get this done soon, have been having sbt issues today though, so couldn't verify my changes.

@johanandren johanandren force-pushed the wip-85-duration-conversion-johanandren branch from 6ecc420 to b9813cb Compare April 30, 2018 09:33
@SethTisue
Copy link
Member

the CI failure is #97 — unrelated to the changes in this PR.

@SethTisue SethTisue merged commit d810c75 into scala:master May 1, 2018
@SethTisue
Copy link
Member

@johanandren would you like me to roll a release...?

@huntc
Copy link

huntc commented May 2, 2018

So... here I was looking at the doc - 15 minutes later I discover that the doc its wrong. :-) I'd like to see a release soon if at possible - thanks.

@johanandren
Copy link
Contributor Author

It isn't urgent for me, but it seems others are interested :)

@johanandren johanandren deleted the wip-85-duration-conversion-johanandren branch May 2, 2018 08:07
@SethTisue
Copy link
Member

SethTisue commented May 2, 2018

@huntc hi Chris! okay I'll roll a release and report here when it's done. probably today

@huntc
Copy link

huntc commented May 2, 2018

Hi @SethTisue - thanks so much. Really appreciated.

@SethTisue
Copy link
Member

0.9.0 (for 2.11 and 2.12 only; skipping 2.13 this time because of #97) is on its way to Maven Central

@huntc
Copy link

huntc commented May 3, 2018

Thanks!

@akara akara mentioned this pull request Feb 27, 2019
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants