Skip to content

Conversation

jakehschwartz
Copy link
Contributor

JAVA-4319

Inserted a default statement for the case class macro, like there is on CaseClassCodec:390

This allows for the -Xlint:strict-unsealed-patmat scalac option to be included with Scala Versions >= 2.13.4.

@jakehschwartz jakehschwartz changed the title Include default match statement to CaseClassCodec macro [JAVA-4319] Include default match statement to CaseClassCodec macro Jan 18, 2022
@jakehschwartz
Copy link
Contributor Author

@jyemin @stIncMale Sorry to ping you directly, and further apologies if you are the wrong people, I just picked active people in the project.

Is there anything I need to add to the PR to get it reviewed?

@jtjeferreira
Copy link
Contributor

Hi @jakehschwartz!
I think would be nice that you added -Xlint:strict-unsealed-patmat to the scalacOptions to make sure this does not happen again...

@jakehschwartz
Copy link
Contributor Author

@jtjeferreira Sure, I could try to figure out how to do that with gradle.

The code I changed is in a Macro, so it gets generated at compile time (of the project that is using the driver, not the driver itself) so it would also probably only get caught if there were tests

@jtjeferreira
Copy link
Contributor

The code I changed is in a Macro, so it gets generated at compile time (of the project that is using the driver, not the driver itself) so it would also probably only get caught if there were tests

Yeah I was assuming those tests exist. Don't forget to also had the option that turns warns into errors

@jakehschwartz
Copy link
Contributor Author

Don't forget to also had the option that turns warns into errors

Done but realized most of the way through fixing them that I had to ignore deprecations

@rozza
Copy link
Member

rozza commented Feb 8, 2022

Hi @jakehschwartz - unfortunately ff02ee2 breaks the build for scala versions 2.11 and 2.12. So I rebased and cherry picked the fixes, without the deprecation changes - See: 178a0d4

Thanks @jakehschwartz for the PR and @jtjeferreira for your feedback on this PR.

Ross

@rozza rozza closed this Feb 8, 2022
@jakehschwartz
Copy link
Contributor Author

@rozza Thanks, I didn't realize but that makes sense. FWIW Scala 2.11 is EOL, it will probably make things easier to support if that is dropped.

Thanks again!

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.

3 participants