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

Proposal: merge cats-time into cats #3766

Closed
diesalbla opened this issue Feb 3, 2021 · 5 comments
Closed

Proposal: merge cats-time into cats #3766

diesalbla opened this issue Feb 3, 2021 · 5 comments

Comments

@diesalbla
Copy link
Contributor

cats-time is a microlibrary that implements instances of kernel typeclasses, such as Show, Ord, Eq, or Hash, for the classes of the java.time package. This package was introduced in version 8.0 of the JDK, in order to replace previous unsatisfactory classes (such as java.Date), and was derived from joda-time.

The proposal is that these files be merged into the kernel package, and the cats-time library archived.

Some considerations on this matter are as follows:

  • Precedent: there is a trait of instances for java.util.UUID already in kernel.
  • java.time is just as standard a library as Option, Either, so it is reasonable to also include its instances in the package.
  • Adding instances for a JDK-specific class would add a coupling between cats and the JVM platform specifically, but as mentioned that already happens with java.util.UUID.
  • This would make the cats kernel incompatible with JDK pre-8 versions. The existing instance for UUID only adds incompatibility with pre-1.5 JDKs. Note that JDK 8 was released in March 2014, almost seven years ago, so I am not sure how far cats retrocompatibility should go.
@DavidGregory084
Copy link
Member

Since cats already requires Java 8+ as of version 2.1.0 because it's only published for Scala 2.12+, I think this proposal is very sensible

@djspiewak
Copy link
Member

The only major concern here would be around ScalaJS support.

@diesalbla
Copy link
Contributor Author

The only major concern here would be around ScalaJS support.

In other projects there is a separation of sources that are JS-specific or JVM-specific. Perhaps we can do the same here?
As noted, this already happens with UUID.

@rossabaker
Copy link
Member

Strong 👍. We shouldn't accept a lesser experience on our flagship platform when a feature is not supported by a secondary platform. And for the Java 8 requirement, public support for Java 7 ended in 2015.

@diesalbla
Copy link
Contributor Author

diesalbla commented Feb 3, 2021

Taking existing support into account, I have opened PR to integrate this into the cats.kernel...

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

No branches or pull requests

4 participants