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

Upgrade to MUnit 1.0.0-M6 #223

Merged
merged 15 commits into from
Aug 2, 2022
18 changes: 9 additions & 9 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
strategy:
matrix:
os: [ubuntu-latest]
scala: [3.0.2, 2.12.16, 2.13.8]
scala: [3.1.2, 2.12.16, 2.13.8]
java: [temurin@8]
project: [rootJS, rootJVM]
runs-on: ${{ matrix.os }}
Expand Down Expand Up @@ -90,11 +90,11 @@ jobs:

- name: Make target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: mkdir -p target ce2/jvm/target .js/target site/target ce3/js/target .jvm/target .native/target ce2/js/target ce3/jvm/target project/target
run: mkdir -p target .js/target site/target core/.js/target core/.jvm/target .jvm/target .native/target project/target

- name: Compress target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
run: tar cf targets.tar target ce2/jvm/target .js/target site/target ce3/js/target .jvm/target .native/target ce2/js/target ce3/jvm/target project/target
run: tar cf targets.tar target .js/target site/target core/.js/target core/.jvm/target .jvm/target .native/target project/target

- name: Upload target directories
if: github.event_name != 'pull_request' && (startsWith(github.ref, 'refs/tags/v') || github.ref == 'refs/heads/main')
Expand Down Expand Up @@ -147,22 +147,22 @@ jobs:
~/Library/Caches/Coursier/v1
key: ${{ runner.os }}-sbt-cache-v2-${{ hashFiles('**/*.sbt') }}-${{ hashFiles('project/build.properties') }}

- name: Download target directories (3.0.2, rootJS)
- name: Download target directories (3.1.2, rootJS)
uses: actions/download-artifact@v2
with:
name: target-${{ matrix.os }}-${{ matrix.java }}-3.0.2-rootJS
name: target-${{ matrix.os }}-${{ matrix.java }}-3.1.2-rootJS

- name: Inflate target directories (3.0.2, rootJS)
- name: Inflate target directories (3.1.2, rootJS)
run: |
tar xf targets.tar
rm targets.tar

- name: Download target directories (3.0.2, rootJVM)
- name: Download target directories (3.1.2, rootJVM)
uses: actions/download-artifact@v2
with:
name: target-${{ matrix.os }}-${{ matrix.java }}-3.0.2-rootJVM
name: target-${{ matrix.os }}-${{ matrix.java }}-3.1.2-rootJVM

- name: Inflate target directories (3.0.2, rootJVM)
- name: Inflate target directories (3.1.2, rootJVM)
run: |
tar xf targets.tar
rm targets.tar
Expand Down
73 changes: 10 additions & 63 deletions build.sbt
Original file line number Diff line number Diff line change
@@ -1,77 +1,24 @@
ThisBuild / tlBaseVersion := "1.0"
ThisBuild / tlBaseVersion := "2.0"
Copy link
Member

Choose a reason for hiding this comment

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

Are we really want to do a new major release based on the fifth milestone?..

Copy link
Member Author

Choose a reason for hiding this comment

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

Bumping the base version doesn't mean a new major release, it means we are starting the 2.x series of development that is not compatible with any 1.x release. I propose we publish this PR as 2.0.0-M1

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I just was confused with this reference to 2.0.0.

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha yes, we always debate the correct deprecation version for http4s too :) I don't really care, I will happily put whatever you want there.


ThisBuild / developers += tlGitHubDev("milanvdm", "Milan van der Meer")
ThisBuild / startYear := Some(2021)

ThisBuild / crossScalaVersions := List("3.0.2", "2.12.16", "2.13.8")

ThisBuild / tlFatalWarningsInCi := false
ThisBuild / crossScalaVersions := List("3.1.2", "2.12.16", "2.13.8")

lazy val docs = project
.in(file("site"))
.dependsOn(ce3.jvm)
.dependsOn(core.jvm)
.enablePlugins(TypelevelSitePlugin)

lazy val root = tlCrossRootProject.aggregate(ce3, ce2)
lazy val root = tlCrossRootProject.aggregate(core)

lazy val ce3 = crossProject(JSPlatform, JVMPlatform)
.crossType(CrossType.Full)
.settings(
name := "munit-cats-effect-3",
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/shared/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/shared/src/test/scala"
)
lazy val core = crossProject(JSPlatform, JVMPlatform)
.crossType(CrossType.Pure)
.in(file("core"))
.settings(
name := "munit-cats-effect",
Copy link
Member Author

Choose a reason for hiding this comment

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

We should make sure to register a Scala Steward migration for munit-cats-effect-3 -> munit-cats-effect.

libraryDependencies ++= Seq(
"org.scalameta" %%% "munit" % "0.7.29",
"org.scalameta" %%% "munit" % "1.0.0-M5",
"org.typelevel" %%% "cats-effect" % "3.3.12"
),
// we are checking binary compatibility from the 1.0.6 version
mimaPreviousArtifacts ~= {
_.filter { m =>
VersionNumber(m.revision).matchesSemVer(SemanticSelector(">=1.0.6"))
}
}
)
.jvmSettings(
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/jvm/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/jvm/src/test/scala"
)
.jsSettings(
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/js/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/js/src/test/scala",
scalaJSLinkerConfig ~= (_.withModuleKind(ModuleKind.CommonJSModule))
)

lazy val ce2 = crossProject(JSPlatform, JVMPlatform)
Copy link
Member

Choose a reason for hiding this comment

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

If we decided to make EOL the whole library ("munit-cats-effect-2"), I argue it should be described in docs. As well as changing munit-cats-effect-3 -> munit-cats-effect. What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it should be described in docs. But if we make those changes in this PR, then it will go live as soon as it merges even before official release. Maybe should make a new branch for me to target my changes to, instead of main?

Copy link
Member

@danicheg danicheg Jun 22, 2022

Choose a reason for hiding this comment

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

I think we already can slice changes on removing CE2 support into another PR and merge it as soon as possible (because no things we should do after it) 👍🏻

Copy link
Member Author

Choose a reason for hiding this comment

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

Good plan, I will prepare it shortly.

Copy link
Member Author

Choose a reason for hiding this comment

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

@danicheg sorry, to clarify: do you mean slice the documentation changes, or do you mean slice 974242f specifically?

I don't think we should drop CE2 from the 1.x series.

Copy link
Member

Choose a reason for hiding this comment

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

Aww. Probably I was wrong here. I was thinking we want to deprecate the whole library. Because we integrate CE support to MUnit, not vice versa. If we encourage investing in migration to CE3, let's do it at max 😆

Copy link
Member Author

Choose a reason for hiding this comment

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

Haha, I see. Actually that was how this library started but it is better it remains independent. Origin story:

Copy link
Member

Choose a reason for hiding this comment

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

Okay, but why not deprecate the CE2-based library? Speaking as one of the http4s maintainers, it'd be totally fine. Because I'm confident, here we integrate precisely CE support.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should just EOL 1.x when 2.x is released. And 2.x won't support CE2 at all.

.crossType(CrossType.Full)
.settings(
name := "munit-cats-effect-2",
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/shared/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/shared/src/test/scala"
)
.settings(
libraryDependencies ++= Seq(
"org.scalameta" %%% "munit" % "0.7.29",
"org.typelevel" %%% "cats-effect" % "2.5.5"
),
// we are checking binary compatibility from the 1.0.6 version
mimaPreviousArtifacts ~= {
_.filter { m =>
VersionNumber(m.revision).matchesSemVer(SemanticSelector(">=1.0.6"))
}
}
)
)
.jvmSettings(
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/jvm/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/jvm/src/test/scala"
)
.jsSettings(
libraryDependencies += "org.scala-js" %%% "scala-js-macrotask-executor" % "1.0.0",
Compile / unmanagedSourceDirectories += baseDirectory.value / "../../common/js/src/main/scala",
Test / unmanagedSourceDirectories += baseDirectory.value / "../../common/js/src/test/scala",
scalaJSLinkerConfig ~= (_.withModuleKind(ModuleKind.CommonJSModule))
)

addCommandAlias("fmt", """scalafmtSbt;scalafmtAll""")
addCommandAlias("fmtCheck", """scalafmtSbtCheck;scalafmtCheckAll""")
30 changes: 0 additions & 30 deletions ce2/js/src/main/scala/munit/CatsEffectSuitePlatform.scala

This file was deleted.

32 changes: 0 additions & 32 deletions ce2/jvm/src/main/scala/munit/CatsEffectSuitePlatform.scala

This file was deleted.

57 changes: 0 additions & 57 deletions ce2/shared/src/main/scala/munit/CatsEffectSuite.scala

This file was deleted.

28 changes: 0 additions & 28 deletions ce3/jvm/src/main/scala/munit/CatsEffectSuitePlatform.scala

This file was deleted.

51 changes: 0 additions & 51 deletions common/jvm/src/main/scala/munit/CatsEffectFixturesPlatform.scala

This file was deleted.

52 changes: 0 additions & 52 deletions common/shared/src/main/scala/munit/CatsEffectFixtures.scala

This file was deleted.

Loading