[GitHub] [kafka] junrao commented on a change in pull request #10638: KAFKA-12758 Added `server-common` module to have server side common classes.

[GitBox <gi...@apache.org>]

junrao commented on a change in pull request #10638:
URL: https://github.com/apache/kafka/pull/10638#discussion_r627548721



##########
File path: build.gradle
##########
@@ -1585,6 +1644,7 @@ project(':shell') {
     implementation libs.jacksonJDK8Datatypes
     implementation libs.jline
     implementation libs.slf4jApi
+    implementation project(':server-common')

Review comment:
       Is this needed since core already depends on server-common?

##########
File path: build.gradle
##########
@@ -1267,11 +1269,13 @@ project(':raft') {
   archivesBaseName = "kafka-raft"
 
   dependencies {
+    implementation project(':server-common')

Review comment:
       Is this needed since server-common is included in metadata?

##########
File path: build.gradle
##########
@@ -1070,6 +1071,7 @@ project(':metadata') {
   archivesBaseName = "kafka-metadata"
 
   dependencies {
+    implementation project(':server-common')

Review comment:
       Not directly related to this PR. Do you know why we only include the storage module in releaseTarGz target, but not other server side modules?

##########
File path: build.gradle
##########
@@ -1345,6 +1349,62 @@ project(':raft') {
   }
 }
 
+project(':server-common') {
+  archivesBaseName = "kafka-server-common"
+
+  dependencies {
+    implementation project(':clients')

Review comment:
       Since server-common depends on clients, perhaps we could remove clients from all modules that depend on server-common?

##########
File path: build.gradle
##########
@@ -1345,6 +1349,62 @@ project(':raft') {
   }
 }
 
+project(':server-common') {
+  archivesBaseName = "kafka-server-common"

Review comment:
       Do we want to move the classes in storage-api here? It helps reduce one module.

##########
File path: build.gradle
##########
@@ -779,6 +779,7 @@ project(':core') {
     api project(':clients')
     api libs.scalaLibrary
 
+    implementation project(':server-common')

Review comment:
       Is that needed since server-common is included in metadata/storage already?




-- 
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.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org