cross-build for 2.13 #287

martijnhoekstra · 2019-08-22T12:56:02Z

No description provided.

martijnhoekstra · 2019-08-22T12:56:23Z

At least bijection-core

nevillelyh · 2019-10-09T18:12:01Z

@martijnhoekstra are you still working on this? I can pick it up if you don't mind.

martijnhoekstra · 2019-10-09T18:55:04Z

Oh, yes, I can pick this up again; I thought this one was ready to go, but maybe it isn't.

nevillelyh · 2019-10-09T18:59:58Z

build.sbt

@@ -39,7 +39,7 @@ def versionFallback(scalaVersion: String, packageVersion210: String, version: St

 val buildLevelSettings = Seq(
  organization := "com.twitter",
-  crossScalaVersions := Seq("2.10.6", "2.11.8", "2.12.1"),
+  crossScalaVersions := Seq("2.10.6", "2.11.8", "2.12.1", "2.13.0"),


Worth updating all of them? I think the latest are 2.10.6, 2.11.12, 2.12.10, & 2.13.1

2.10.7, I think

build.sbt

nevillelyh · 2019-10-09T19:09:02Z

Also we should probably get the CI build green.
@johnynek worth adding to scala-steward in the future?

johnynek · 2019-10-09T23:49:56Z

Yeah, I think scala-steward is a win for all repos.

Per twitter/bijection#287 (comment)

martijnhoekstra · 2019-10-10T14:13:26Z

scala-steward-org/repos#28

martijnhoekstra · 2019-10-10T14:32:32Z

Oh, it turns out I had lots of half-baked stuff laying around for bijection. There was lots of stuff that needed fixing, and I'm not sure what order I had in mind for that. Maybe first we can see what scala-steward brings to the table.

nevillelyh · 2019-10-10T15:40:17Z

@martijnhoekstra what about a quick fixup on this PR so we can get it in and let scala-steward pick up the rest?

martijnhoekstra · 2019-10-10T15:47:41Z

@nevillelyh if it indeed turns out to be a quick fixup I'm all for it. When I get home tonight I can work on that, and see if it is and the problem is just scalafmt, or if there is more going on.

martijnhoekstra · 2019-10-10T18:11:18Z

quick update: this is far farther from home than I had hoped.

johnynek · 2019-10-10T18:26:18Z

I'd like to use the strategy of making a scala-2.12- directory and scala-2.13+ directory or something for any changes. Since we haven't published for 2.13, anything goes, but for 2.11/2.12 I'd really like to not break binary compatibility.

We did something like this here:
typelevel/paiges#189

martijnhoekstra · 2019-10-10T18:33:04Z

that sounds good @johnynek -- even without the bincompat argument it'll be necessary.

martijnhoekstra · 2019-10-10T18:40:56Z

I burned this branch to the ground, and started it with a new blank slate. Marked the PR WIP.

martijnhoekstra · 2019-10-11T16:30:26Z

bijection-core/src/test/scala/com/twitter/bijection/CollectionLaws.scala

+    //things do work out. Without, we get diverging expansion starting at 
+    //Injection.fromImplicitBijection
+    implicit val i1 = Bijection.fromInjection(Injection.int2String)
+    def ibij: ImplicitBijection[CA, CB] = implicitly[ImplicitBijection[List[Int], Vector[String @@ Rep[Int]]]] 


@johnynek I pushed my WIP, as I'm hitting a hurdle here: scala 2.13 runs out of heap trying to get some sort of LUB here (I haven't figured out what it's trying to LUB). Ideas/sage advice/sharp insight more than welcome.

The whole A @@ Rep[B] trick is very rarely used. In general, most cases would use Injection[A, B] It may be that something with the tricks to do @@

https://github.com/twitter/bijection/blob/develop/bijection-core/src/main/scala/com/twitter/bijection/Rep.scala

https://github.com/twitter/bijection/blob/develop/bijection-core/src/main/scala/com/twitter/package.scala#L55

johnynek · 2019-10-11T21:34:39Z

bijection-core/src/test/scala/com/twitter/bijection/CollectionLaws.scala

+    //if we start here with Bijection.fromInjection(Injection.int2String)
+    //things do work out. Without, we get diverging expansion starting at 
+    //Injection.fromImplicitBijection
+    implicit val i1 = Bijection.fromInjection(Injection.int2String)


it could help to put a type here, maybe Bijection[Int, String @@ Rep[Int]]?

This keeps getting weirder. With

implicit val i1 = Bijection.fromInjection(Injection.int2String) def ibij: ImplicitBijection[CA, CB] = implicitly[ImplicitBijection[CA, CB]]

it succeeds fine.

With def ibij: ImplicitBijection[CA, CB] = implicitly[ImplicitBijection[List[Int], Vector[String @@ Rep[Int]] it crashes the compiler with OOM. That's obviously a compiler bug, but it's not immediately clear to me if, and if so how we can work around.

martijnhoekstra · 2019-10-13T12:56:59Z

scala/bug#11770 messes up bijection-clojure. Worked around by calling generated static java methods.

bijection-core/src/test/java/com/twitter/bijection/TestBijectionInJava.java

martijnhoekstra · 2019-10-14T08:47:57Z

bijection-core/src/test/scala/com/twitter/bijection/TypeclassBijectionLaws.scala

-  implicit val numericTypeclassBijection: TypeclassBijection[Numeric] =
-    new TypeclassBijection[Numeric] {
-      def apply[A, B](tc: Numeric[A], bij: Bijection[A, B]) =
-        new Numeric[B] {
-          def plus(x: B, y: B) = bij(tc.plus(bij.invert(x), bij.invert(y)))
-          def minus(x: B, y: B) = bij(tc.minus(bij.invert(x), bij.invert(y)))
-          def times(x: B, y: B) = bij(tc.times(bij.invert(x), bij.invert(y)))
-          def negate(x: B) = bij(tc.negate(bij.invert(x)))
-          def fromInt(x: Int) = bij(tc.fromInt(x))
-          def toInt(x: B) = tc.toInt(bij.invert(x))
-          def toLong(x: B) = tc.toLong(bij.invert(x))
-          def toFloat(x: B) = tc.toFloat(bij.invert(x))
-          def toDouble(x: B) = tc.toDouble(bij.invert(x))
-          def compare(x: B, y: B) = tc.compare(bij.invert(x), bij.invert(y))
-        }
-    }
-


Extending numeric requires an extra member in 2.13. Probably not worth it to split this test up, as typeclass testing seems adequately tested in other tests. Does it need additional replacements?

bijection-core/src/test/java/com/twitter/bijection/TestBijectionInJava.java

build.sbt

martijnhoekstra · 2019-10-15T13:29:46Z

bijection-json/src/main/scala/com/twitter/bijection/json/JsonInjection.scala

@@ -73,7 +71,7 @@ trait LowPriorityJson {
  * You need to import all the methods of this object to get general
  * Injection[T,JsonNode] to work
  */
-object JsonNodeInjection extends LowPriorityJson with java.io.Serializable {
+object JsonNodeInjection extends CollectionJson with LowPriorityJson with java.io.Serializable {


This approach, with version specific parts mixed in to a version-neutral "main" can also be applied to the other split files. The upside is less duplicated code. The downside is more complexity. Both are backward binary compatible.

Happy to do it either way.

https://github.com/twitter/bijection/compare/develop...martijnhoekstra:213_unsplit?expand=1 is the "keep as much as possible together" approach

martijnhoekstra · 2019-10-16T06:52:53Z

Rebased to fix conflicts in tests

CLAassistant · 2019-11-16T23:49:29Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

martijnhoekstra · 2019-12-14T10:34:30Z

rebased

chrisbenincasa · 2019-12-31T21:50:13Z

Any update on the status of this? This is blocking the conversion of the memcached module over in #771 (which seems to be the last dependencyless module)

martijnhoekstra · 2020-01-02T20:56:10Z

If I remember right this is ready for review

dispalt · 2020-01-10T18:02:24Z

This looks good anything keeping this from merging?

nevillelyh · 2020-01-13T15:24:46Z

LGTM and MiMa is green too. @johnynek wanna take a final look?

johnynek · 2020-01-13T18:25:28Z

This is great! Thanks for all the hard work!

martijnhoekstra · 2020-01-14T20:13:57Z

Nice! Is a release also possible?

johnynek · 2020-01-14T22:48:02Z

Just released:

https://github.com/twitter/bijection/releases/tag/v0.9.7

martijnhoekstra force-pushed the 213 branch from 4b36596 to 75ef046 Compare August 22, 2019 12:58

nevillelyh reviewed Oct 9, 2019

View reviewed changes

martijnhoekstra added a commit to martijnhoekstra/repos that referenced this pull request Oct 10, 2019

add - twitter/bijection

3cb7d41

Per twitter/bijection#287 (comment)

martijnhoekstra mentioned this pull request Oct 10, 2019

add - twitter/bijection scala-steward-org/repos#28

Merged

martijnhoekstra force-pushed the 213 branch 2 times, most recently from 2027265 to d0ae146 Compare October 10, 2019 14:27

martijnhoekstra force-pushed the 213 branch from d0ae146 to e2ea34f Compare October 10, 2019 18:39

martijnhoekstra changed the title ~~cross-build for 2.13~~ WIP: cross-build for 2.13 Oct 10, 2019

martijnhoekstra commented Oct 11, 2019

View reviewed changes

johnynek reviewed Oct 11, 2019

View reviewed changes

martijnhoekstra force-pushed the 213 branch 3 times, most recently from 0aba187 to 1ca6489 Compare October 14, 2019 08:44

martijnhoekstra commented Oct 14, 2019

View reviewed changes

build.sbt Outdated Show resolved Hide resolved

martijnhoekstra force-pushed the 213 branch from 594e3bb to 0c8260d Compare October 14, 2019 14:11

martijnhoekstra force-pushed the 213 branch from 9704ad4 to cb6e8a7 Compare October 15, 2019 11:25

martijnhoekstra commented Oct 15, 2019

View reviewed changes

martijnhoekstra force-pushed the 213 branch from cb6e8a7 to 1174826 Compare October 16, 2019 06:52

martijnhoekstra force-pushed the 213 branch 4 times, most recently from 6a01c7d to 1a77bc8 Compare October 18, 2019 18:15

h-vetinari mentioned this pull request Nov 5, 2019

Scala 2.13.x twitter/algebird#671

Closed

Martijn Hoekstra added 7 commits December 13, 2019 13:00

rebooting 213 port

6cdcb01

bijection-core: cross build 213

deabf10

bijection-json split 213

a32ed41

bijection-util: use Future.apply instead of concurrent.future

8397725

bijection-clojure: codegen updated for scalabug 11770

e46f57c

bijection-json: re-unifiy shared code

5457fb5

bijection-core: unify Bufferable

5263f73

martijnhoekstra force-pushed the 213 branch from 1a77bc8 to 5263f73 Compare December 14, 2019 10:33

chrisbenincasa mentioned this pull request Dec 14, 2019

Build for 2.13.0 twitter/finagle#771

Closed

28 tasks

dispalt mentioned this pull request Jan 9, 2020

2.13 goodcover/tagless-redux#49

Closed

2 tasks

yufangong mentioned this pull request Jan 9, 2020

finagle-memcached: -> 2.13 twitter/finagle#823

Closed

johnynek merged commit f1aaa9d into twitter:develop Jan 13, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cross-build for 2.13 #287

cross-build for 2.13 #287

martijnhoekstra commented Aug 22, 2019

martijnhoekstra commented Aug 22, 2019

nevillelyh commented Oct 9, 2019

martijnhoekstra commented Oct 9, 2019

nevillelyh Oct 9, 2019

mosesn Oct 10, 2019

nevillelyh commented Oct 9, 2019 •

edited

Loading

johnynek commented Oct 9, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019 •

edited

Loading

nevillelyh commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

johnynek commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra Oct 11, 2019

johnynek Oct 11, 2019

johnynek Oct 11, 2019

martijnhoekstra Oct 12, 2019

martijnhoekstra commented Oct 13, 2019 •

edited

Loading

martijnhoekstra Oct 14, 2019

martijnhoekstra Oct 15, 2019

martijnhoekstra Oct 16, 2019

martijnhoekstra commented Oct 16, 2019

CLAassistant commented Nov 16, 2019

martijnhoekstra commented Dec 14, 2019

chrisbenincasa commented Dec 31, 2019

martijnhoekstra commented Jan 2, 2020

dispalt commented Jan 10, 2020

nevillelyh commented Jan 13, 2020

johnynek commented Jan 13, 2020

martijnhoekstra commented Jan 14, 2020

johnynek commented Jan 14, 2020

cross-build for 2.13 #287

cross-build for 2.13 #287

Conversation

martijnhoekstra commented Aug 22, 2019

martijnhoekstra commented Aug 22, 2019

nevillelyh commented Oct 9, 2019

martijnhoekstra commented Oct 9, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nevillelyh commented Oct 9, 2019 • edited Loading

johnynek commented Oct 9, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019 • edited Loading

nevillelyh commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

johnynek commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

martijnhoekstra commented Oct 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martijnhoekstra commented Oct 13, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

martijnhoekstra commented Oct 16, 2019

CLAassistant commented Nov 16, 2019

martijnhoekstra commented Dec 14, 2019

chrisbenincasa commented Dec 31, 2019

martijnhoekstra commented Jan 2, 2020

dispalt commented Jan 10, 2020

nevillelyh commented Jan 13, 2020

johnynek commented Jan 13, 2020

martijnhoekstra commented Jan 14, 2020

johnynek commented Jan 14, 2020

nevillelyh commented Oct 9, 2019 •

edited

Loading

martijnhoekstra commented Oct 10, 2019 •

edited

Loading

martijnhoekstra commented Oct 13, 2019 •

edited

Loading