snazy commented on code in PR #523:
URL: https://github.com/apache/polaris/pull/523#discussion_r1878317790
##########
gradle/projects.main.properties:
##########
@@ -19,7 +19,8 @@
#
polaris-core=polaris-core
-polaris-service=polaris-service
+polaris-service-common=polaris-service/common
Review Comment:
My thought on "common":
Generally I'm not a fan of any "common module" (it's in almost all projects
I know a sink hole for all kinds of different things). I'd be in favor of
refactoring this and split it out into separate modules by concern.
But granted that it's currently very hard to split things by concern, we
have to do it later.
##########
gradle/projects.main.properties:
##########
@@ -19,7 +19,8 @@
#
polaris-core=polaris-core
-polaris-service=polaris-service
+polaris-service-common=polaris-service/common
+polaris-service-dropwizard=polaris-service/dropwizard
Review Comment:
I'd prefer
```suggestion
polaris-dropwizard-service=dropwizard/service
```
so there's room for additional framework specific modules.
##########
polaris-core/build.gradle.kts:
##########
@@ -64,7 +64,6 @@ dependencies {
}
}
- implementation(libs.javax.inject)
Review Comment:
I noticed `javax.inject` and wanted to remove it as well - thanks for this!
##########
polaris-service/common/build.gradle.kts:
##########
@@ -50,34 +46,23 @@ dependencies {
implementation(libs.hadoop.hdfs.client)
implementation(libs.smallrye.common.annotation)
- implementation(platform(libs.dropwizard.bom))
Review Comment:
Nice removal of dependencies :)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
