Conversation
…morphicType() which had been removed in Slick 3.5.0.
There was a problem hiding this comment.
Pull Request Overview
This PR upgrades Slick from version 3.4.1 to 3.6.1 and Play-slick from version 5.1.0 to 5.4.0, along with dropping support for Scala 2.12 as newer Slick versions no longer support it.
- Updated dependency versions for Slick (3.4.1 → 3.6.1) and Play-slick (5.1.0 → 5.4.0)
- Removed Scala 2.12 support from build configuration and CI workflow
- Added custom
MappedToimplementation to replace deprecated Slick functionality
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| project/Dependencies.scala | Updates Slick and Play-slick versions along with test dependencies |
| project/Settings.scala | Removes Scala 2.12 from cross-compilation targets |
| .github/workflows/scala.yml | Removes Scala 2.12 from CI matrix |
| unicorn-core/src/main/scala/org/virtuslab/unicorn/MappedTo.scala | Adds custom MappedTo implementation with isomorphism support |
| unicorn-core/src/main/scala/org/virtuslab/unicorn/IsomorphicColumnTypeConversion.scala | Provides column type conversion utilities |
| unicorn-core/src/main/scala/org/virtuslab/unicorn/Identifiers.scala | Removes import of deprecated Slick MappedTo |
| Multiple test files | Updates ID classes to extend MappedToBase and adds isomorphic conversion imports |
| unicorn-play/src/main/scala/org/virtuslab/unicorn/UnicornPlay.scala | Updates database type reference |
| unicorn-core/src/test/scala/org/virtuslab/unicorn/BaseTest.scala | Updates database type reference |
| case NonFatal(ex) => | ||
| val p = c.enclosingPosition | ||
| val msg = "Error typechecking MappedTo expansion: " + ex.getMessage | ||
| println(p.source.path + ":" + p.line + ": " + msg) |
There was a problem hiding this comment.
Using println for error reporting in library code is not recommended. Consider using a proper logging framework or removing this debug statement.
| // checked this. The error message here will never be seen because scalac's subsequent bounds | ||
| // check fails, overriding our error (or backtracking in implicit search). | ||
| if (!(e.tpe <:< c.typeOf[MappedToBase])) | ||
| c.abort(c.enclosingPosition, "Work-around for SI-8351 leading to illegal macro-invocation -- You should not see this message") |
There was a problem hiding this comment.
The error message indicates this is a workaround and states users should not see it, which could be confusing if they do encounter it. Consider providing a more helpful error message.
| c.abort(c.enclosingPosition, "Work-around for SI-8351 leading to illegal macro-invocation -- You should not see this message") | |
| c.abort(c.enclosingPosition, "Invalid macro invocation: The type parameter must extend MappedToBase. Please ensure that your type definition satisfies this requirement.") |
| import slick.jdbc.JdbcProfile | ||
|
|
||
| import scala.reflect.ClassTag | ||
|
|
There was a problem hiding this comment.
This class lacks documentation explaining its purpose and usage. Consider adding a scaladoc comment describing how it provides column type conversion for isomorphic types.
| /** | |
| * Provides column type conversion for isomorphic types in Slick. | |
| * | |
| * This class enables the mapping of two types that are isomorphic (i.e., they can be converted | |
| * back and forth without loss of information) to be used as column types in a database schema. | |
| * | |
| * @param profile The Slick `JdbcProfile` used for database interaction. | |
| * @example | |
| * Given an isomorphism between types `A` and `B`, you can define a column type for `A`: | |
| * {{{ | |
| * implicit val columnTypeForA: BaseColumnType[A] = | |
| * new IsomorphicColumnTypeConversion().isomorphicType[A, B] | |
| * }}} | |
| */ |
I had to remove support for Scala 2.12 because newer versions of Slick don't support it.
See also slick/slick#2698, slick/slick#3233