Simplify module structure and introduce alleycats-retry #132
Conversation
|
Should probably release a new major version after this and #131 (if we decide on merging this). |
build.sbt
Outdated
| crossScalaVersions := scalaVersions, | ||
| libraryDependencies ++= Seq( | ||
| "org.typelevel" %%% "cats-core" % catsVersion, | ||
| "org.typelevel" %%% "cats-effect" % "2.0.0", |
|
I had a quick look at the Maven Central download stats for last month (not a very accurate measure of how the lib is actually being used, but it's the only data we have). Combining Scala 2.11, 2.12 and 2.13 and JVM/JS:
So, with the caveat that we can't really be sure what these numbers mean, they could be interpreted to say that around 85% of people who use cats-retry use it with cats-effect. Given that, I think it's reasonable to promote cats-effect to a core dependency and demote Also +1 for removing the Monix module. As Monix is based on cats-effect it doesn't need any special integration. |
| The `alleycats-retry` module provides instances for `cats.Id`, `cats.Eval` and `Future` that | ||
| simply do a `Thread.sleep(...)`. | ||
|
|
||
| To use them, simply import `retry.alleycats.instances`: |
There was a problem hiding this comment.
The import is missing a ._ on the end.
Also let's include a line here showing how to add a dependency on the alleycats module. It's a bit of a mouthful:
```scala mdoc:passthrough
println(
s"""
|```
|libraryDependencies += "com.github.cb372" %% "alleycats-retry" % "${retry.BuildInfo.version.replaceFirst("\\+.*", "")}"
|```
|""".stripMargin.trim
)
| @@ -148,10 +144,6 @@ val root = project | |||
| .aggregate( | |||
| coreJVM, | |||
| coreJS, | |||
| catsEffectJVM, | |||
There was a problem hiding this comment.
Shouldn't we aggregate alleycatsJVM and alleycatsJS here?
| @@ -67,9 +68,13 @@ val core = crossProject(JVMPlatform, JSPlatform) | |||
| .in(file("modules/core")) | |||
| .settings(moduleSettings) | |||
| .settings( | |||
| name := "cats-retry", | |||
There was a problem hiding this comment.
This will set the module name to cats-retry-cats-retry 😄
Let's remove the moduleName := ... line from moduleSettings above.
| @@ -98,8 +98,6 @@ import cats.effect.Timer | |||
| import scala.concurrent.ExecutionContext.global | |||
There was a problem hiding this comment.
Please update the instructions higher up in this file on how to add the dependency in sbt. The cats-effect module is going away and the cats-retry-core module is changing name to just cats-retry (right?).
By the way that change of artifact name is going to mess with scala-steward. That's a shame, but not much we can do about it. We could publish a duplicate artifact as cats-retry-core, but we wouldn't want to do that forever. Eventually people will need to migrate.
There was a problem hiding this comment.
We're pushing some breaking changes, so it's not the worst thing :)
|
Thanks for the review! :) I'll probably get to this tomorrow |
This pulls the
cats-effectinstance intocoreand moves theFuture,IdandEvalinstances toalleycats-retry. It also removes themonixmodule, since it's already covered byTimer. :)