RussellSpitzer commented on code in PR #1229:
URL: https://github.com/apache/polaris/pull/1229#discussion_r2023714172
##########
bom/build.gradle.kts:
##########
@@ -33,6 +33,7 @@ dependencies {
api(project(":polaris-immutables"))
api(project(":polaris-misc-types"))
api(project(":polaris-version"))
+ api(project(":polaris-persistence-varint"))
Review Comment:
Basically my thought is that Varint serialization is not a core competency
of the Polaris project, it's just a utility that is specific to the current
approach's NoSql implementation. I think it's ok to get in if it's just
something that the Mongo impl wants to use in a separate codebase, but if this
is something we wanted to use generally I would strongly lean towards finding
an existing library rather than rolling our own.
I think we would have a different discussion if this was a analytics engine,
or something that obviously needed it's own varint impl, but i'm not sure why a
Catalog needs it's own varint impl. That said, I'm willing to include one if
it's isolated from the rest of the code base.
As for modules, I don't think there really should be an issue with keeping
the whole Mongo impl in it's own module. I think at least everything should be
in the same Java package unless there is a proven need to expose a public api
to the rest of the project. That's just my gut feeling here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscr...@polaris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
