Skip to content

[3.3.x]: Scala 2.13.0-RC2 - #2031

Merged
octonato merged 19 commits into
slick:3.3.xfrom
marcospereira:backport/3.3.x/2023-scala-2.13.0-RC1
May 28, 2019
Merged

[3.3.x]: Scala 2.13.0-RC2#2031
octonato merged 19 commits into
slick:3.3.xfrom
marcospereira:backport/3.3.x/2023-scala-2.13.0-RC1

Conversation

@marcospereira

@marcospereira marcospereira commented May 2, 2019

Copy link
Copy Markdown
Contributor

Backports #2023, #2018 and #2012 to branch 3.3.x.

So that we avoid breaking binary compatibility and ensures that other projects using Slick can also cross-build using Scala 2.13.0 doing a patch upgrade.

Status

WIP since this is still using scala-collections-compat 1.0.0, but stable libraries should instead use 2.0.0. See note here: https://github.com/scala/scala-collection-compat#compatibility-library

@marcospereira

Copy link
Copy Markdown
Contributor Author

/cc @SethTisue. :-)


def +=(s: String): SQLBuilder

def sep[T](sequence: scala.collection.immutable.Iterable[T], separator: String)(f: T => Unit): Unit = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In the current master branch we use Iterable which expands to scala.collection.Iterable
Is there any reason to choose for the immutable variant specifically?

If we do chose for scala.collection.Iterable for 2.13 then for binary compatibility we can use the deprecated Traversable type alias, see https://github.com/scala/scala/blob/2.13.x/src/library/scala/package.scala#L51

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

In the current master branch we use Iterable which expands to scala.collection.Iterable
Is there any reason to choose for the immutable variant specifically?

No reason. Let's stick with scala.collection.Iterable.

If we do chose for scala.collection.Iterable for 2.13 then for binary compatibility we can use the deprecated Traversable type alias, see https://github.com/scala/scala/blob/2.13.x/src/library/scala/package.scala#L51

I suggest then to forward port some of the changes here with deprecation notice, and not deprecate in a patch version of 3.3.x where Scala 2.12 is fully supported and there is no reason to deprecate methods using Traversable.

WDYT?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remember that 3.3.x must be binary compatible with 3.3.0 for scala 2.12 and 2.11.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suggest then to forward port some of the changes here with deprecation notice, and not deprecate in a patch version of 3.3.x where Scala 2.12 is fully supported and there is no reason to deprecate methods using Traversable.

Do you mean the changes where Traversable(Once) is replaced by Iterable(Once)?

If using separate source files for pre- and post 2.13 can be avoided when we definately should. For this PR where we must maintain binary compatibility we can use Traversable and TraversableOnce. In scala 2.13 these are aliases for Iterable and IterableOnce.

To some extent I agree with the point that the type change from Traversable to Iterable might require deprecation warnings. However since nearly all Traverable subtypes are also Iterable, it probably won't cause any source-compatibility problems in practise, and if so these problems should not be hard to fix

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Remember that 3.3.x must be binary compatible with 3.3.0 for scala 2.12 and 2.11.

Yep, this is what I'm trying to do in this PR. So far, it looks like MiMa checks are happy with the changes here. :-)

Do you mean the changes where Traversable(Once) is replaced by Iterable(Once)?

I mean, for the 3.3.x branch, have both supported and avoid deprecating the methods using Traversable since this type is not deprecated in Scala 2.12/2.11. For master, we can have overloads to support both and deprecate the version using Traversable. But I don't have a strong opinion here, and we can go for Iterable without deprecating as you said.

Anyway, it is out of the scope for this PR, I think. Here my only objective is to have 3.3.x artifacts for Scala 2.13.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I agree that the deprecation is out of scope for this PR.

However the original point I was trying to make was that we can use the same code for all scala versions. In that case we should use the Traversable/TraversableOnce type.
In scala 2.13 Traversable is a type alias for Iterable.

Not having to maintain separate sources for the different scala versions keeps the build as simple as possible.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@WellingR, got it. Sounds like a good plan to me.

@marcospereira

Copy link
Copy Markdown
Contributor Author

@WellingR, see the new commits based on your review.

Thanks!

@marcospereira

Copy link
Copy Markdown
Contributor Author

Depends on #2040.

@marcospereira marcospereira changed the title [WIP]: Backport/3.3.x/2023 scala 2.13.0 rc1 [3.3x]: Scala 2.13.0-RC2 (wip) May 22, 2019
@marcospereira marcospereira added this to the 3.3.1 milestone May 22, 2019
* Scala 2.13.0-RC2

* Update scalatest version in samples
@marcospereira

Copy link
Copy Markdown
Contributor Author

#2040 cherry-picked here.

@WellingR I think this is ready to be merged.

@marcospereira marcospereira changed the title [3.3x]: Scala 2.13.0-RC2 (wip) [3.3x]: Scala 2.13.0-RC2 May 22, 2019
@dwijnand dwijnand changed the title [3.3x]: Scala 2.13.0-RC2 [3.3.x]: Scala 2.13.0-RC2 May 23, 2019
@octonato

Copy link
Copy Markdown
Contributor

Ok, I'm planning to merge this one and cut 3.3.1-RC1 with support for Scala 2.13.0-RC2.

I will do it later tonight unless someone blocks me! :-)

@octonato
octonato merged commit a966604 into slick:3.3.x May 28, 2019
@marcospereira
marcospereira deleted the backport/3.3.x/2023-scala-2.13.0-RC1 branch May 28, 2019 18:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants