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

Make boilerplate syntax classes extend Serializable #3439

Merged

Conversation

ceedubs
Copy link
Contributor

@ceedubs ceedubs commented May 28, 2020

Today I ran into a serializability issue when using mapN in a Spark
environment 😬.

Today I ran into a serializability issue when using `mapN` in a Spark
environment 😬.
@ceedubs ceedubs force-pushed the boilerplate-syntax-serializable branch from 9ff69f6 to 5a0ef40 Compare May 28, 2020 23:52
@codecov-commenter
Copy link

codecov-commenter commented May 29, 2020

Codecov Report

Merging #3439 into master will decrease coverage by 0.20%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3439      +/-   ##
==========================================
- Coverage   91.64%   91.43%   -0.21%     
==========================================
  Files         381      380       -1     
  Lines        8269     8244      -25     
  Branches      227      227              
==========================================
- Hits         7578     7538      -40     
- Misses        691      706      +15     
Flag Coverage Δ
#scala_version_212 ?
#scala_version_213 91.43% <ø> (ø)
Impacted Files Coverage Δ
core/src/main/scala/cats/syntax/semigroupal.scala 50.00% <ø> (ø)
core/src/main/scala/cats/data/OneAnd.scala 79.10% <0.00%> (-16.49%) ⬇️
core/src/main/scala/cats/Reducible.scala 95.65% <0.00%> (-2.18%) ⬇️
core/src/main/scala/cats/syntax/either.scala 81.61% <0.00%> (-1.72%) ⬇️
...rc/main/scala/cats/laws/discipline/arbitrary.scala 98.29% <0.00%> (-0.86%) ⬇️
core/src/main/scala/cats/Show.scala 67.56% <0.00%> (-0.86%) ⬇️
core/src/main/scala/cats/instances/try.scala 86.44% <0.00%> (-0.45%) ⬇️
core/src/main/scala/cats/data/Tuple2K.scala 91.30% <0.00%> (-0.19%) ⬇️
.../src/main/scala/cats/laws/discipline/MiniInt.scala 97.22% <0.00%> (-0.08%) ⬇️
core/src/main/scala/cats/data/NonEmptyChain.scala 92.92% <0.00%> (-0.08%) ⬇️
... and 10 more

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 a0f3534...5a0ef40. Read the comment docs.

@travisbrown
Copy link
Contributor

Definite 👍 to the boilerplate additions. For SemigroupalOps, though, wouldn't it be better to do this at the Simulacrum level, for all Ops traits? I've just done that in #3440 via this Simulacrum Scalafix change.

@travisbrown
Copy link
Contributor

By the way if anyone else is annoyed by scrolling past the huge Codecov comment in PRs like this, #3428 fixes that, and just needs one more review to merge.

@djspiewak
Copy link
Member

in a Spark environment

Pretty sure I just diagnosed the problem here. ;-)

@travisbrown
Copy link
Contributor

Merging this now. I'll include changing back the SemigroupalOps addition when I rebase #3440.

@travisbrown travisbrown self-requested a review May 30, 2020 11:28
@travisbrown travisbrown merged commit 452de16 into typelevel:master May 30, 2020
@travisbrown travisbrown added this to the 2.2.0-M3 milestone May 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants