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.

Sign up for GitHub

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

Jump to bottom

Update to scala 2.13.6 #456

Merged
merged 8 commits into from
May 24, 2021
Merged

Update to scala 2.13.6 #456

merged 8 commits into from
May 24, 2021

Conversation

regadas
Copy link
Collaborator

@regadas regadas commented Jul 14, 2020

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jul 14, 2020 •
edited
Loading

Codecov Report

Merging #456 (e1ee7aa) into develop (78ca722) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop     #456   +/-   ##
========================================
  Coverage    91.69%   91.69%           
========================================
  Files           42       42           
  Lines         1396     1397    +1     
  Branches        35       36    +1     
========================================
+ Hits          1280     1281    +1     
  Misses         116      116
Impacted Files Coverage Δ
...cala/com/twitter/chill/ScalaKryoInstantiator.scala 99.27% <100.00%> (+<0.01%) ⬆️

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 78ca722...e1ee7aa. Read the comment docs.

@regadas regadas force-pushed the update/scala-2.13.3 branch 2 times, most recently from 6bd3631 to 5c621f9 Compare July 21, 2020 08:48
@regadas
Copy link
Collaborator Author

regadas commented Jul 22, 2020

@johnynek any extra thoughts on this?

@johnynek
Copy link
Collaborator

one super frustrating thing about this repo is that we try to tell people that they shouldn't use kryo (or chill) for long term serialization (although we don't seem clear about that in the readme, so I guess how would people know that), yet almost every change can break them and we have weak tests for compatibility.

I really wish we could just pick an approach: keep compatibility, or say we are never for long term serialization and each release can break binary serializations.

It makes reviewing pretty tough and anxiety provoking.

See #467 for a recent concern from a user.

@regadas
Copy link
Collaborator Author

regadas commented Sep 3, 2020

Hi @johnynek! sorry for late reply! Keeping binary serialization compat will require a lot of effort and a decent test stack as you mentioned.

I'm leaning towards 2 approaches

  1. We state that we don't guarantee on any release.

  2. We do best effort approach where:

  • we don't guarantee between scala versions
  • we try to keep it between patches and minors but we don't guarantee on majors.
  • we don't guarantee on scala version updates (as we know things can change between patches). Maybe here we can go with, on each version update we do a major release.
  • we state that long term storage is not recommended and the above.

wdyt?

@johnynek
Copy link
Collaborator

related to #514

I think there is a case to be made to drop any guarantee that serialized data is compatible between releases.

What do you think?

@regadas
Copy link
Collaborator Author

regadas commented Jan 16, 2021 •
edited
Loading

Hi @johnynek sorry for late reply. Yea it seems there is. I'm gonna update this PR and add notice to the README.

edit: I also share your concerns and I agree with you.

@regadas regadas changed the title Update to scala 2.13.3 Update to scala 2.13.4 Jan 18, 2021
regadas
regadas commented Jan 18, 2021
View reviewed changes
README.md
@@ -9,6 +9,9 @@ Extensions for the [Kryo serialization library](https://github.com/EsotericSoftw
serializers and a set of classes to ease configuration of Kryo in systems like Hadoop, Storm,
Akka, etc.

### Compatibility
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@johnynek added this section. what do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Circling back on this. Does this seem ok?

regadas
regadas commented Jan 18, 2021
View reviewed changes
build.sbt Outdated
@@ -175,7 +175,7 @@ lazy val chill = Project(
).settings(sharedSettings)
.settings(
name := "chill",
crossScalaVersions += "2.13.1",
crossScalaVersions += "2.13.4",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Update this to reflect the latest 2.13.4

@nicknezis
Copy link

What is the status of this PR? Can we bump Scala from 2.13.4 to 2.13.5 and merge? Or is it blocked by something?

@regadas regadas changed the title Update to scala 2.13.4 Update to scala 2.13.6 May 21, 2021
@regadas regadas mentioned this pull request May 21, 2021
Closed
@regadas regadas merged commit 8114daa into twitter:develop May 24, 2021
@regadas regadas deleted the update/scala-2.13.3 branch May 24, 2021 07:50
regadas added a commit that referenced this pull request May 24, 2021
@regadas
Fix update to scala 2.13.6 (#456)
06e90ad
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@johnynek johnynek johnynek left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@regadas @codecov-commenter @johnynek @nicknezis