Grant Henke has posted comments on this change.

Change subject: KUDU-2066. Add experimental Gradle build support
......................................................................


Patch Set 7:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/shadow.gradle
File java/gradle/shadow.gradle:

Line 21: tasks.remove(knows)  // Remove "easter egg" knows task.
> what's this? tried googling but couldn't find it
The person who wrote the gradle shadow plugin added a task that just outputs 
ascii art. I didn't want it cluttering `gradle tasks` output and resulting in 
questions.


Line 49: )
> not understanding this part at all
This plugin assumes you are shading all dependencies. In the kudu-client we 
only shade a few so we need a way to signify that. I added a configuration 
called "compileUnshaded" to track this information, but need to map it back to 
a maven scope for publishing. Otherwise these dependencies would not show up in 
the published maven poms. 

I can definitely add more docs around this and link to issue on the plugin 
tracking improvements so this isn't necessary.


http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradle/wrapper/gradle-wrapper.jar
File java/gradle/wrapper/gradle-wrapper.jar:

> the ASF is against checking in binary jars
My understanding is that this is okay as long as they are not included in 
distributions. The samza project also doing this: 
https://github.com/apache/samza/tree/master/gradle/wrapper

There is some discussion on it on SAMZA-283 too. 

We could consider some other method of downloading this as a sort of "custom 
bootstrap".


http://gerrit.cloudera.org:8080/#/c/7258/7/java/gradlew.bat
File java/gradlew.bat:

Line 1: @if "%DEBUG%" == "" @echo off
> can we add these files to the RAT excludes file?
Done


http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestTimeouts.java:

Line 35:   @Ignore
> why is this change in here? if you want to disable a test please do it in a
This test is super flaky and failed my build almost all the time. I didn't mean 
to check this in, but its probably something worth fixing.


http://gerrit.cloudera.org:8080/#/c/7258/7/java/kudu-spark/build.gradle
File java/kudu-spark/build.gradle:

Line 40:       } else {
> is it possible to be explicit about == "2" and then give a build error if i
Done


Line 50: } else {
> same
I think the pattern would likely stay the same for any major spark build going 
forward. The only reason this exists is because spark 1 uses "spark" instead of 
"spark1" in the artifact name.


-- 
To view, visit http://gerrit.cloudera.org:8080/7258
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib257cdd019a1f383c886b9238bb47d96576c4421
Gerrit-PatchSet: 7
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Adar Dembo <a...@cloudera.com>
Gerrit-Reviewer: Dan Burkert <danburk...@apache.org>
Gerrit-Reviewer: Grant Henke <granthe...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-HasComments: Yes

Reply via email to