-
Notifications
You must be signed in to change notification settings - Fork 155
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
Update scala-library to 2.12.14 #562
Update scala-library to 2.12.14 #562
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #562 +/- ##
========================================
Coverage 91.69% 91.69%
========================================
Files 42 42
Lines 1397 1397
Branches 36 36
========================================
Hits 1281 1281
Misses 116 116 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WrappedArrayBuilder
in 2.12.13+ suffered some updates (still need look into more detail) and as a consequence, the WrappedArray
round trip serde is broken; From a quick look, it seems that the result array from WrappedArrayBuilder
is losing its class.
I wonder why we are using the WrappedArrayBuilder
which has an underlying array copy instead of just WrappedArray.make
; With the latter, serde roundtrip is ok.
@johnynek I'm hoping you can shed some light, I might be missing something, do you remember?
chill/chill-scala/src/main/scala-2.12-/com/twitter/chill/WrappedArraySerializer.scala
Lines 34 to 39 in e067ea8
val clazz = kser.readObject(in, classOf[Class[T]]) | |
val array = kser.readClassAndObject(in).asInstanceOf[Array[T]] | |
val bldr = new WrappedArrayBuilder[T](ClassTag[T](clazz)) | |
bldr.sizeHint(array.size) | |
bldr ++= array | |
bldr.result() |
I'm pretty ready to give up on the idea that serialized data is stable. The use of this library is for runtime ad-hoc serialization between processes (think: akka, hadoop, spark, storm, etc...) It's not for archival storage. That's a much harder problem. What do you think @regadas ? |
@johnynek I'm totally on board with that! based on your previous suggestion (older PR's) I added a section to the README. https://github.com/twitter/chill#compatibility let me know if it needs some work. |
sounds good! Let's go nuts... As long as CI passes. :) We probably should work to make it easier to test serializers with scalacheck and get our coverage up high. |
Updates org.scala-lang:scala-library from 2.12.12 to 2.12.14.
I'll automatically update this PR to resolve conflicts as long as you don't change it yourself.
If you'd like to skip this version, you can just close this PR. If you have any feedback, just mention me in the comments below.
Configure Scala Steward for your repository with a
.scala-steward.conf
file.Have a fantastic day writing Scala!
Files still referring to the old version number
The following files still refer to the old version number (2.12.12).
You might want to review and update them manually.
Ignore future updates
Add this to your
.scala-steward.conf
file to ignore future updates of this dependency:labels: library-update, semver-patch, old-version-remains