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

Configure shared and test modules to cross-build scala 3 #676

Merged
merged 10 commits into from
Nov 7, 2023

Conversation

RustedBones
Copy link
Contributor

cross build scala 3 test and shared modules

Some breaking changes:

  • Derivation must me moved to dedicated class in specific scala folder
  • All type-classes created with magnolia are created with gen under the type-class companion object
    • AnnotationType.gen[T]: Annotationtype[T]
    • EnumType.gen[T]: EnumType[T]

Thos should probably be merged into a v0.7.x branch to get started

@RustedBones RustedBones mentioned this pull request Jan 18, 2023
21 tasks
@codecov
Copy link

codecov bot commented Jan 18, 2023

Codecov Report

Merging #676 (cbef5c2) into main (171077e) will increase coverage by 0.16%.
Report is 60 commits behind head on main.
The diff coverage is 97.91%.

@@            Coverage Diff             @@
##             main     #676      +/-   ##
==========================================
+ Coverage   95.05%   95.21%   +0.16%     
==========================================
  Files          52       51       -1     
  Lines        1860     1777      -83     
  Branches      181      175       -6     
==========================================
- Hits         1768     1692      -76     
+ Misses         92       85       -7     
Files Coverage Δ
avro/src/main/scala/magnolify/avro/AvroType.scala 97.10% <ø> (ø)
...src/main/scala/magnolify/avro/unsafe/package.scala 100.00% <100.00%> (ø)
...c/main/scala/magnolify/bigquery/TableRowType.scala 95.29% <100.00%> (ø)
.../scala/magnolify/bigquery/TimestampConverter.scala 100.00% <100.00%> (ø)
...main/scala/magnolify/bigquery/unsafe/package.scala 100.00% <100.00%> (ø)
...scala/magnolify/cats/semiauto/BandDerivation.scala 87.50% <ø> (ø)
...ify/cats/semiauto/CommutativeGroupDerivation.scala 93.75% <ø> (ø)
...fy/cats/semiauto/CommutativeMonoidDerivation.scala 91.66% <ø> (ø)
...cats/semiauto/CommutativeSemigroupDerivation.scala 87.50% <ø> (ø)
...cala/magnolify/cats/semiauto/GroupDerivation.scala 86.20% <ø> (ø)
... and 13 more

@RustedBones RustedBones marked this pull request as ready for review April 27, 2023 08:54
java: [corretto@11, corretto@8]
exclude:
- scala: 3.2.1
Copy link
Contributor

Choose a reason for hiding this comment

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

looks like this is no longer needed, after Java 8 deprecation PR is merged

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This file is now generated from build.sbt and CI fails if it detects it is not in sync.
When we'll update the branch with changes in main, we'll just need to run sbt githubWorkflowGenerate to have this updated.


- name: Inflate target directories (3.2.1)
run: |
tar xf targets.tar
Copy link
Contributor

Choose a reason for hiding this comment

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

What exactly targets.tar will contain?

build.sbt Outdated
// tolerate some nested macro expansion
"-Ymax-inlines",
"-Xmax-inlines",
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it related to new Magnolia version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, those are required by magnolia with scala 3. Some macros are generating too long inlines.

build.sbt Outdated
@@ -186,14 +186,15 @@ val commonSettings = Seq(
tlFatalWarningsInCi := false,
tlJdkRelease := Some(8),
tlSkipIrrelevantScalas := true,
scalacOptions ~= { _.filterNot(_ == "-source:3.0-migration") },
Copy link
Contributor

Choose a reason for hiding this comment

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

can you explain what this option means?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

source:3.0-migration instructs the scala 3 compiler to run in migration mode. This is the default from sbt-typlevel when cross-building with scala 2 (the sources are the same).
However, with this option, the compiler fails on the dedicated scala-3 folder because sources are not in the scala 2 style. See discussion scala/scala3#16720

build.sbt Outdated
@@ -296,6 +297,8 @@ lazy val scalacheck = project
commonSettings,
moduleName := "magnolify-scalacheck",
description := "Magnolia add-on for ScalaCheck",
crossScalaVersions := Seq(scala213, scala212),
Copy link
Contributor

Choose a reason for hiding this comment

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

so we still don't want to cross-build with scala3?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not all projects yet. Only shared and test

@@ -0,0 +1,50 @@
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

all code in scala-2 was just copied?

package magnolify

import scala.util.hashing.MurmurHash3
import scala.collection.compat.Factory
Copy link
Contributor

Choose a reason for hiding this comment

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

is it really needed in scala3? I thought it is only 2.12 .vs 2.13 issue

Copy link
Contributor Author

Choose a reason for hiding this comment

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

scala 2.13 and scala 3 have the same collection IIRC, but we still need the shim layer to get the code cross-build with scala 2.12. I think this can be removed when we stop 2.12 support.

@RustedBones RustedBones force-pushed the update-magnolia-scala3 branch 5 times, most recently from c01656a to a03e5d5 Compare August 23, 2023 07:52
@RustedBones RustedBones merged commit e3eec4a into main Nov 7, 2023
11 checks passed
@RustedBones RustedBones deleted the update-magnolia-scala3 branch November 7, 2023 14:10
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.

None yet

3 participants