Add support for 2.13.0-M3 #2203
Conversation
|
Notes on build failures:
|
|
@tpolecat, @eperinan For the build to work, I had to explicitly add the see https://github.com/typelevel/cats/pull/2203/files#diff-a84f91f20f040218bccd09fed4761fb3R16 |
Codecov Report
@@ Coverage Diff @@
## master #2203 +/- ##
=======================================
Coverage 95.05% 95.05%
=======================================
Files 333 333
Lines 5789 5789
Branches 211 211
=======================================
Hits 5503 5503
Misses 286 286Continue to review full report at Codecov.
|
|
@tpolecat, @eperinan , here's a printout :
|
ghost
dismissed
|
Rebased on master, as that was blocked |
|
@BennyHill why is it that this also includes 1.1 release notes, author updates, etc? |
I rebased on master. see before - merge was blocked |
|
Hmm if the changes were just brought in from master then they shouldn't be showing up in the GitHub file diff though, should they? I'm a little confused as to what's going on here. |
|
Ok, I'll do a squash in the morning to fix them up |
|
Hmm, looks the same in #2200 ? |
|
Done, ready for review |
ghost
mentioned this pull request
There was a problem hiding this comment.
Thanks a bunch, @BennyHill! I left a couple of minor comments, but this generally looks good to me.
build.sbt
Outdated
| case Some((2, 13)) => | ||
| "org.scalatest" %%% "scalatest" % scalatest_2_13 | ||
| case _ => | ||
| "org.scalatest" %%% "scalatest" % scalaTestVersion |
There was a problem hiding this comment.
This logic is duplicated in two places. Should we create a def scalatestVersion(scalaVersion: String): String or something?
There was a problem hiding this comment.
They are not quite the same, the first has a the test scope. A "real" function looks more like this or as in catalysts. So I opted for a comment instead i,e // 2.13.0-M3 workaround
So as this is commented as an interim workaround, is this OK for now?
There was a problem hiding this comment.
I can also add a link to the comment describing this interim workaround - scalatest/scalatest#1321 (comment)
There was a problem hiding this comment.
@BennyHill I opened a PR against your repo with the workaround that I was thinking might clean things up a little bit. Feel free to merge if you like it: https://github.com/BennyHill/cats/pull/1
.travis.yml
Outdated
| - 2.13.0-M3 | ||
|
|
||
| matrix: | ||
| allow_failures: |
There was a problem hiding this comment.
It might be helpful to leave a comment here about why this is happening.
build.sbt
Outdated
| @@ -401,7 +420,7 @@ lazy val bench = project.dependsOn(macrosJVM, coreJVM, freeJVM, lawsJVM) | |||
| .settings(commonJvmSettings) | |||
| .settings(coverageEnabled := false) | |||
| .settings(libraryDependencies ++= Seq( | |||
| "org.scalaz" %% "scalaz-core" % "7.2.15")) | |||
| "org.scalaz" %% "scalaz-core" % "7.2.19")) | |||
There was a problem hiding this comment.
7.2.20 could be used here. Very minor though
There was a problem hiding this comment.
Yep, small point but the small points tend to add up over time. Besides, might as well add this whilst doing other changes - thx for catching this 👍
But allow to fail on 2.13.0-M3 see