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

Update scala-library to 2.12.14 #562

Merged

Conversation

scala-steward
Copy link
Contributor

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.

.github/workflows/ci.yaml
Ignore future updates

Add this to your .scala-steward.conf file to ignore future updates of this dependency:

updates.ignore = [ { groupId = "org.scala-lang", artifactId = "scala-library" } ]

labels: library-update, semver-patch, old-version-remains

@codecov-commenter
Copy link

codecov-commenter commented May 28, 2021

Codecov Report

Merging #562 (1aa7d18) into develop (9b19e3a) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           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.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b19e3a...1aa7d18. Read the comment docs.

Copy link
Collaborator

@regadas regadas left a 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?

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()

@johnynek
Copy link
Collaborator

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 ?

@regadas
Copy link
Collaborator

regadas commented Jun 1, 2021

@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.

@johnynek johnynek merged commit 50f10ed into twitter:develop Jun 2, 2021
@johnynek
Copy link
Collaborator

johnynek commented Jun 2, 2021

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.

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.

4 participants