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

Add FiniteDuration instances #2517

Closed
LukaJCB opened this issue Sep 19, 2018 · 8 comments · Fixed by #2544
Closed

Add FiniteDuration instances #2517

LukaJCB opened this issue Sep 19, 2018 · 8 comments · Fixed by #2544

Comments

@LukaJCB
Copy link
Member

LukaJCB commented Sep 19, 2018

We currently have instances for Duration only, which means that you can use FiniteDuration with it, but must upcast it, which can be annoying:

3.minutes |+| 3.minutes // Fails to compile because `|+|` is not a member of `FiniteDuration`.
@kun-song
Copy link
Contributor

I will do it :)

@LukaJCB
Copy link
Member Author

LukaJCB commented Sep 19, 2018

Awesome, thank you @satansk.
You can basically reuse the Duration implementations already present in cats-kernel :)

@adelbertc
Copy link
Contributor

I use FiniteDurations all the time, Durations not so much 💯

@LukaJCB LukaJCB added this to the 1.5 milestone Sep 22, 2018
@calvinbrown085
Copy link
Contributor

@satansk If you're still working on this feel free to keep going :) Can we just reuse https://github.com/typelevel/cats/blob/master/kernel/src/main/scala/cats/kernel/instances/duration.scala

With FiniteDuration instead of `Duration?

Where do these instances go? @codecentric

Thanks!

@LukaJCB
Copy link
Member Author

LukaJCB commented Sep 29, 2018

Yes the instances should be exactly the same :) and you should be able to place them analogously to where the duration instances are placed

@calvinbrown085
Copy link
Contributor

I opened a PR here, @satansk if you wanted to work on this i'm more than happy to close it!

@kun-song
Copy link
Contributor

@calvinbrown085 go ahead :)

I'm busy for a release in company, and I was planning to work on this on my 7 days vocation one day later. Since you already have made a PR, just keep on working on it.

@calvinbrown085
Copy link
Contributor

@satansk Sounds good! Just making sure I don't trample on anyone :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants