[GitHub] [beam] lukecwik commented on a change in pull request #13075: [BEAM-9444] Use GCP BOM to set package versions

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

lukecwik commented on a change in pull request #13075:
URL: https://github.com/apache/beam/pull/13075#discussion_r508082875



##########
File path: examples/java/build.gradle
##########
@@ -48,6 +48,7 @@ configurations.sparkRunnerPreCommit {
 }
 
 dependencies {
+  compile platform(library.java.google_cloud_platform_libraries_bom)

Review comment:
       Should we be using enforcedPlatform instead of platform everywhere?
   
   Before this change, we get enforcedPlatform like behavior because in BeamModulePlugin.groovy we are explicitly forcing version overrides.
   
   Migrating to enforcedPlatform would force the remainder of versions within the BOM to be used though which you haven't done as part of this PR (e.g. guava being 29.0-android).
   
   I don't see any gradle documentation that explains how an enforcedPlatform and a `force` `resolutionStrategy`  behave together.

##########
File path: buildSrc/src/main/groovy/org/apache/beam/gradle/BeamModulePlugin.groovy
##########
@@ -21,6 +21,8 @@ package org.apache.beam.gradle
 import com.github.jengelman.gradle.plugins.shadow.tasks.ShadowJar
 import groovy.json.JsonOutput
 import groovy.json.JsonSlurper
+import java.util.concurrent.atomic.AtomicInteger

Review comment:
       I don't think this is necessary as well.




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