[GitHub] [beam] jrmccluskey commented on a diff in pull request #17216: Simplify specifying additional dependencies in Go SDK in XLang IOs

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

jrmccluskey commented on code in PR #17216:
URL: https://github.com/apache/beam/pull/17216#discussion_r843974997


##########
sdks/go/pkg/beam/core/runtime/xlangx/registry.go:
##########
@@ -276,6 +278,16 @@ func UseAutomatedJavaExpansionService(gradleTarget string) string {
 	return autoJavaNamespace + Separator + gradleTarget
 }
 
+// AddClasspaths takes an expansion address string and creates a tagged string
+// suffixed with classpath separator and classpaths provided. This indicates that
+// the classpaths should be added to expansion service JAR before auto start-up.
+//
+// Intended for use by cross language wrappers to add classpaths to the manifest of
+// an expansion service in case of automated start-up of JAVA expansion service.
+func AddClasspaths(expansionAddr string, classpaths []string) string {

Review Comment:
   I'm not super into having both functions that need to be called together when there isn't necessarily a clear relationship between the two. If this only gets used in the automated case then I can see it being added as an option, but if there's stand-alone utility than the comments just need to be clear (and maybe have a small example explaining what's going on for doc purposes)



-- 
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: github-unsubscribe@beam.apache.org

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