-
Notifications
You must be signed in to change notification settings - Fork 348
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
Fix encoders and decoders for java.util.UUID. #665
Conversation
@getquill/maintainers Do you have any idea how to override encoder and decoder for specific |
Postgres jdbc accepts UUID as object because postgres supports |
Should I make separate |
@getquill/maintainers Done. |
|
||
import java.util.UUID | ||
|
||
trait UuidObjectCodecs { |
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.
UUIDEncoding
would be more consistent with the codebase
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.
or UUIDObjectEncoding
. Just saw the string version
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.
You're right 👍
@mxl sorry for the late feedback, I'm on vacation. The approach with specific context types looks good, but I'd like to avoid the sub-module "explosion". Considering that the specific contexts don't require more dependencies, wdyt about keeping all specific contexts under |
@fwbrasil I have several pros for keeping separate sub-modules:
|
@mxl thanks for the clarification, I'm still not convinced, though.
I think most of the Android developers use something like proguard to remove unused classes/resources.
They should require only a test dependency, the published jar wouldn't have additional dependencies, right?
The same is true if we keep a single sub-module.
That's a good point, I agree that we should split the codebase into multiple sbt projects and allow different lifecycles for each module. For instance, we could have a new module that starts with version I'd like to hear from the other maintainers, this is an important change. @getquill/maintainers |
Yes. I thought that default proguard setup does not shrink unused classes but it's the opposite.
Yes, but why is it done like that? Other Quill modules dependencies are not test-only (
Agree. Copypasting single directory is a little easier but it's a weak argument. Actually when #379 will be implemented there will no need to duplicate tests so there will be only
+1. I originally thought about splitting git repo but I agree that it will complicate things a lot. Summarizing the above said, I just want us to develop a single approach to module organization for the case when we have some generic driver ( |
It's because
It's intentional but it's not hacky in any way. There's no need to depend on a specific vendor implementation, we only need the abstraction that jdbc provides.
They don't provide a unified interface for the different databases, that's the only reason to keep them separate. It'd be great if we could merge them. |
@fwbrasil You're right. I have not thought about that. I've rolled back those changes and left only |
@mxl awesome! :D |
Fixes #594
Temporary workaround for #535 (only for JDBC)
Problem
SqlContext
.java.util.UUID
conversion to some other type likeString
.JdbcContext
for specificSqlIdiom
..quill-jdbc
depends on driver libraries for all dialects which is not necessary when using one specific dialectSolution
Split.quill-jdbc
to separate projects for each dialectJdbcContext
abstract and remove defaultuuidEncoder
anduuidDecoder
.JdbcContext
for each dialect with mixed-in trait with customuuidEncoder
anduuidDecoder
.Checklist
README.md
if applicable[WIP]
to the pull request title if it's work in progresssbt scalariformFormat test:scalariformFormat
to make sure that the source files are formatted@getquill/maintainers