You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@geode.apache.org by GitBox <gi...@apache.org> on 2021/09/13 22:47:44 UTC

[GitHub] [geode] rhoughton-pivot commented on a change in pull request #6863: GEODE-9595: Exclude circular build dependencies

rhoughton-pivot commented on a change in pull request #6863:
URL: https://github.com/apache/geode/pull/6863#discussion_r707767966



##########
File path: geode-wan/build.gradle
##########
@@ -62,7 +64,9 @@ dependencies {
 
   upgradeTestImplementation(project(':geode-gfsh'))
   upgradeTestImplementation(project(':geode-junit'))
-  upgradeTestImplementation(project(':geode-dunit'))
+  upgradeTestImplementation(project(':geode-dunit')) {

Review comment:
       geode-dunit does not bring in geode-wan

##########
File path: geode-web-api/build.gradle
##########
@@ -104,6 +104,7 @@ dependencies {
   }
   integrationTestImplementation(project(':geode-dunit')) {
     exclude module: 'geode-core'
+    exclude module: 'geode-web-api'

Review comment:
       geode-dunit does not bring in geode-web-api.

##########
File path: geode-core/build.gradle
##########
@@ -233,7 +233,9 @@ dependencies {
         exclude group: '*'
     }
 
-    runtimeOnly(project(':geode-deployment:geode-deployment-legacy'))
+    runtimeOnly(project(':geode-deployment:geode-deployment-legacy')) {
+        exclude module: 'geode-core'

Review comment:
       What is the actual problem being addressed by this change?
   
   `geode-core` needs `geode-deployment:geode-deployment-legacy`, which needs `geode-core` to exist on the classpath. At runtime, this will be true. Why not declare it?

##########
File path: geode-management/build.gradle
##########
@@ -49,7 +49,9 @@ dependencies {
   testCompileOnly(platform(project(':boms:geode-all-bom')))
   testCompileOnly('io.swagger:swagger-annotations')
 
-  integrationTestImplementation(project(':geode-dunit'))
+  integrationTestImplementation(project(':geode-dunit')) {

Review comment:
       geode-dunit needs geode-core, which needs geode-management. What is the actual concern in the testing here? 

##########
File path: geode-wan/build.gradle
##########
@@ -45,7 +45,9 @@ dependencies {
 
 
   distributedTestImplementation(project(':geode-gfsh'))
-  distributedTestImplementation(project(':geode-dunit'))
+  distributedTestImplementation(project(':geode-dunit')) {
+    exclude module: 'geode-wan'

Review comment:
       geode-dunit does not bring in geode-wan
   
   




-- 
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: notifications-unsubscribe@geode.apache.org

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