mockitoguy commented on a change in pull request #2998:
URL: https://github.com/apache/incubator-gobblin/pull/2998#discussion_r430506149
##########
File path: .travis.yml
##########
@@ -42,6 +43,9 @@ env:
- RUN_TEST_GROUP=default
- RUN_TEST_GROUP=group1
- RUN_TEST_GROUP=coverage
+ global:
+ - secure:
f7aIQDgpWHIkYf19mtViOLwH/zBAeVuqOQtu9G4cfDl6R+lqYbTuSEA+HyP2EzT9E7w2bPdVRuwUEb3/FFCBcHTjSBiQ+Nnhtto1RvoXjYlzV9ImG/5s4dsYsrUGWyWxoWXHNRFLE8Xeq4pe6lHR/Md+MajuxdBxgd8AFefMvTEW7a9h8cc+w+kEkKZ9msyiJTzxiTy7d9Yob6bNOEfhTAIUk5jK+a9qqYjDILaps75Y6BMcI3fQZyZno3+/8KBNhk7ncQKf4voRwmQRcAaVM0isQ2kUW4MIRykXRQT3zZJcXxYUoNThn0T9RydjzSu14EM5iNY9IdJVTsojihI/Fvfac9K24UAV5d/k8vlrnTJ4feJfDR1LoHNtCa72U8/cuEKFu2WoJg+kmukcW3pOAshRVLaZw7eCUMS+HevG1uG2wLISN3lAyVoqJCyvu+HaoD1wWfnwfxdZxUXDi+T+Pv7niqVshXMRF8lRQ3+Xf52b40+xhJ78SUWDQgAfClQrJ52o8xEnGOskBAoaJGLlChPMFWhbgHZ8f0cRKPrW/+Q8KGkrcyoo/yxauMkdkiXmg2h0ebNZSfgRc4gn79mU0BKjmsZPKw9RqifGijjAfc0zPplMJMle0hplnuX0QvFALA0go2LInTCvYofk+PU/BMlDHCHJVEBiAarMXv/8P0w=
Review comment:
(non blocking suggestion) I recommend keeping the environmental
variables in the Travis web app. You can specify them if you have admin rights
to the project. @vikrambohra, please request admin rights to gobbling project.
##########
File path: gobblin-rest-service/gobblin-rest-api/build.gradle
##########
@@ -112,6 +112,67 @@ artifacts {
archives dataTemplateSourcesJar, dataTemplateJavadocJar,
restClientSourcesJar, restClientJavadocJar
}
+publishing {
+ publications {
+ dataTemplate(MavenPublication) {
+ //from components.java
+ artifactId dataTemplateName
+ artifacts = [artifact(tasks.mainDataTemplateJar),
dataTemplateSourcesJar, dataTemplateJavadocJar]
Review comment:
a bit simpler and should work:
```
artifacts = [mainDataTemplateJar, dataTemplateSourcesJar,
dataTemplateJavadocJar ]
```
##########
File path: gobblin-binary-management/build.gradle
##########
@@ -25,9 +25,9 @@ dependencies {
compile externalDependency.avroMapredH2
compile externalDependency.guava
compile externalDependency.hadoopHdfs
- runtime externalDependency.hadoopCommon
- runtime externalDependency.hadoopClientCore
- runtime externalDependency.hadoopAuth
+ runtimeOnly externalDependency.hadoopCommon
Review comment:
Is there a reason all dependencies are mapped into "runtimeOnly"? I
suspect that what you really want here is "implementation" (it will be mapped
to Maven "runtime" scope in the poms). See
https://docs.gradle.org/current/userguide/java_library_plugin.html#sec:java_library_separation
##########
File path: gradle.properties
##########
@@ -21,7 +21,7 @@ org.gradle.daemon=true
# Configures only relevant projects to speed up the configuration of large
projects
# Useful when specific project/task is invoked
-org.gradle.configureondemand=true
+org.gradle.configureondemand=false
Review comment:
Any reason for this change?
##########
File path: gobblin-rest-service/gobblin-rest-api/build.gradle
##########
@@ -112,6 +112,67 @@ artifacts {
archives dataTemplateSourcesJar, dataTemplateJavadocJar,
restClientSourcesJar, restClientJavadocJar
}
+publishing {
+ publications {
+ dataTemplate(MavenPublication) {
+ //from components.java
+ artifactId dataTemplateName
+ artifacts = [artifact(tasks.mainDataTemplateJar),
dataTemplateSourcesJar, dataTemplateJavadocJar]
+ pom pomAttributes
+ pom.withXml {
+ def dependenciesNode = asNode().appendNode('dependencies')
+
+ configurations.runtime.allDependencies.each {
Review comment:
This seems to be duplicated in multiple build files. You can extract a
method somewhere and use it to reduce the duplication, something like:
```
addRuntimeDependenciesToPom(pom)
```
To reuse build logic you can use the same techinque as with the existing
"pomAttributes" method
##########
File path: gobblin-binary-management/build.gradle
##########
@@ -25,9 +25,9 @@ dependencies {
compile externalDependency.avroMapredH2
compile externalDependency.guava
compile externalDependency.hadoopHdfs
- runtime externalDependency.hadoopCommon
- runtime externalDependency.hadoopClientCore
- runtime externalDependency.hadoopAuth
+ runtimeOnly externalDependency.hadoopCommon
Review comment:
> so, those dependencies where "runtime" before I changed it to
"runtimeOnly" because maven-publish wasn't mapping them into the pom file.
Understood. "implementation" is also mapped to "runtime" scope in poms.
Please read the Gradle docs to understand the difference between
"implementation" and "runtimeOnly". If you want to continue to use
"runtimeOnly" for all those dependencies, I'm OK.
##########
File path: gobblin-modules/gobblin-orc-dep/build.gradle
##########
@@ -62,4 +63,15 @@ shadowJar {
relocate 'com.google', 'shadow.gobblin.orc.com.google'
}
+publishing {
+ publications {
+ shadowPub(MavenPublication) {
+ artifacts = [shadowJar]
+ pom pomAttributes
+ pom.withXml addRuntimeDependenciesToPom
Review comment:
Great!
##########
File path: gobblin-binary-management/build.gradle
##########
@@ -25,9 +25,9 @@ dependencies {
compile externalDependency.avroMapredH2
compile externalDependency.guava
compile externalDependency.hadoopHdfs
- runtime externalDependency.hadoopCommon
- runtime externalDependency.hadoopClientCore
- runtime externalDependency.hadoopAuth
+ runtimeOnly externalDependency.hadoopCommon
Review comment:
Understood! You're right and "runtimeOnly" is the right path forward.
Thanks!
----------------------------------------------------------------
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:
us...@infra.apache.org
