Scala 3 migration #158
Scala 3 migration #158
Conversation
…to ashevc/scala-3-cross-compile # Conflicts: # dsl/src/main/scala/com/crobox/clickhouse/dsl/column/Magnets.scala
dsl/src/main/scala/com/crobox/clickhouse/dsl/misc/DSLImprovements.scala
Outdated
Show resolved
Hide resolved
dsl/src/main/scala/com/crobox/clickhouse/dsl/OperationalQuery.scala
Outdated
Show resolved
Hide resolved
| @@ -3,7 +3,7 @@ package com.crobox.clickhouse.dsl | |||
| import scala.util.Try | |||
|
|
|||
| trait Table { | |||
| val database: String | |||
| def database: String = "" | |||
There was a problem hiding this comment.
Just curious, why did you instantiate this to an empty string? In the previous scenario compile errors would popup if not set. Now there might be a chance that an empty database is selected (which should be avoided imho)
| def greater(col1: ConstOrColMagnet[_], col2: ConstOrColMagnet[_]): ExpressionColumn[Boolean] = ComparisonColumn(col1 , ">", col2) | ||
| def lessOrEquals(col1: ConstOrColMagnet[_], col2: ConstOrColMagnet[_]): ExpressionColumn[Boolean] = ComparisonColumn(col1 , "<=", col2) | ||
| def greaterOrEquals(col1: ConstOrColMagnet[_], col2: ConstOrColMagnet[_]): ExpressionColumn[Boolean] = ComparisonColumn(col1 , ">=", col2) | ||
| def _equals(col1: ConstOrColMagnet[_], col2: ConstOrColMagnet[_]): ExpressionColumn[Boolean] = ComparisonColumn(col1, "=", col2) |
There was a problem hiding this comment.
I really don't like the _ (which was already there btw). Can't we get rid of it? Or make this one deprecated and add the 'equals()' method. If compiler starts complaining, perhaps rename it to: 'isEquals' or something similar?
| // ComparabeWith trait and Cast case class were members of ComparisonFunctions and TypeCastFunctions trait | ||
| // respectively. But placing them in the mixin traits causes Scala 3 compiler to crash. Hence, placing these | ||
| // constructs here is a workaround allowing for the codebase to be compiled with Scala 3. | ||
| trait ComparableWith[M <: Magnet[_]] { |
There was a problem hiding this comment.
In scala 3 the [_] should be rewritten to [?] right?
There was a problem hiding this comment.
@lwolters Eventually in the future, yes. See https://dotty.epfl.ch/docs/reference/changed-features/wildcards.html but for now it would be too big of a hassle to change
| @@ -120,7 +154,7 @@ trait Magnets { | |||
| with StringSearchOps | |||
| with EmptyNonEmptyCol[C] | |||
|
|
|||
| implicit def stringColMagnetFromString[T <: String: QueryValue](s: T): StringColMagnet[String] = | |||
| implicit def stringColMagnetFromString[T <: String : QueryValue](s: T): StringColMagnet[String] = | |||
There was a problem hiding this comment.
The space between String and QueryValue doesn't seem right. Looks like formatting issues
This migration is verified by running tests coverage.
Changes in this PR:
Castcase class andComparableWithtrait needed to be moved from mixin traits to Magnet trait, otherwise Scala 3 compiler crash occurredEDIT:
Additional changes in tests:
Due to errors that specifically occured in the CI/CD build, the following changes has been added to tests
===method in dsl tests replaced byisEq, as===was evaluated to Booleanscala-2folder as it relies on extension methodsInOpsmethods implicit conversions were not applied properly. There was a need to explicitly specify type annotation for the fields on whichinmethods were applied. This may require more deep work on this in the future to figure out the root cause of the issue