Add initial version of Gradle build files by AndrewMBurke · Pull Request #403 · googleads/google-ads-java

AndrewMBurke · 2021-02-23T17:44:35Z

This is part of the effort to address long build times for the Java library, such as is raised in #346.

nwbirnie · 2021-02-23T19:07:50Z

+
+description = 'Google Ads API client library for Java examples'
+
+public class ExampleRunnerTask extends JavaExec {


Is this class only needed to allow the user to specify without the com.google.... prefix? If so is there perhaps an easier way to do this? The gradle build files are so pretty, it's a shame to put java boilerplate in them ;-)

This seemed the best way to address the fact that this project has multiple "Main" classes.

The better, more Gradle-y way would be to have a single main class and to build the subproject as a Java application. Then you could just call ./gradlew run --args.... This would, of course, involve some refactoring of the project.

Could we move this to the buildSrc directory to simplify the build file here?

Doing so would then apply the task to all subprojects, i.e. calling ./gradlew runExample ... would run this code for the annotation processor, library, examples, and migration examples subprojects. You could instead call ./gradlew google-ads-examples:runExample ..., but that seems like an extra inconvenience when the task only applies to the one subproject.

See related response to Josh's question below.

jradcliff · 2021-02-23T21:39:55Z

+
+description = 'Google Ads API client library for Java examples'
+
+public class ExampleRunnerTask extends JavaExec {


Is there a way to have this for the migration examples project as well, without duplication?

Good point, I didn't explicitly test those. I'll adjust and test.

The migration examples should of course still be runnable from IntelliJ.

Will test moving the task code into the top-level build files, then filtering for which subproject to apply them to.

Restructured; task code is now in buildSrc, and there are now two executable tasks: runExample, as before, and runMigrationExample.

jradcliff · 2021-02-23T21:41:36Z

+dependencies {
+    implementation 'com.squareup:javapoet:1.11.1'
+    implementation 'javax.annotation:javax.annotation-api:1.3.2'
+    implementation 'com.google.api:gax:1.50.1'


Does gax need to be here, given it's in java-conventions?

I actually added this in only to match the dependencies from the Maven POM files:

google-ads-java/google-ads-annotation-processing/pom.xml

Line 66 in e94a364

<version>1.50.1</version>

. This one is 1.50.1, while the rest are 1.60.1.

This line can be removed if this subproject doesn't need to be pegged to 1.50.1.

The effective version here for users will be the one from the main project (since it's shallower in the dependency hierarchy). We should just remove this (so we're on 1.60.1).

nwbirnie · 2021-02-24T11:56:50Z

+dependencies {
+    implementation 'com.squareup:javapoet:1.11.1'
+    implementation 'javax.annotation:javax.annotation-api:1.3.2'
+    implementation 'com.google.api:gax:1.50.1'


The effective version here for users will be the one from the main project (since it's shallower in the dependency hierarchy). We should just remove this (so we're on 1.60.1).

nwbirnie · 2021-02-24T12:08:23Z

+    @TaskAction
+    @Override
+    public void exec() {
+        if (!(exampleArguments?.trim())) {


We (sadly) need to support Java 8 which doesn't have the ? operator. Can you convert this to an explicit null check (sorry!)

This is actually Groovy code, not Java. It compiles fine in Ubuntu with Java 8 as the only available JDK.

If you specify javac -source 8 -target 8 does this still compile?

I added a flag to target Java 1.8 compatibility of the output, just to be safe.

Not sure why would we use javac here?

nwbirnie · 2021-02-24T12:10:24Z

+
+description = 'Google Ads API client library for Java examples'
+
+public class ExampleRunnerTask extends JavaExec {


Could we move this to the buildSrc directory to simplify the build file here?

nwbirnie · 2021-02-24T12:47:32Z

+            main = basePackage + this.exampleArguments
+        }
+        // Otherwise, separate the input and set the arguments to pass to the main class.
+        else {


Please run google-java-format on the java code.

This is actually Groovy code; I'll apply Java style to it though.

Reformatted, please check.

nwbirnie

Couple of nits, otherwise LGTM. Nice job!

nwbirnie · 2021-02-25T10:33:03Z

+task runMigrationExample(type: ExampleRunnerTask) {
+        classpath = sourceSets.main.runtimeClasspath
+        basePackage = 'com.google.ads.googleads.migration.'
+    }


nit: extra whitespace

nwbirnie · 2021-02-25T10:33:13Z

+    testImplementation 'io.grpc:grpc-context:1.30.0'
+    testImplementation 'com.google.truth:truth:0.27'
+    testImplementation 'com.google.auto.value:auto-value-annotations:1.7.3'
+


nit: extra whitespace

nwbirnie · 2021-02-25T12:02:18Z

@@ -0,0 +1,26 @@
+/*


One more: please add build directories and .gradle to .gitignore.

nwbirnie · 2021-03-12T15:45:16Z

Friendly ping for @jradcliff :)

jradcliff

By the way, on my Linux box:

a build from scratch took just 2m59s
after that (with the daemon running), clean build took 2s to 12s

That's quite an impressive improvement.

Add initial version of Gradle build files

f890796

AndrewMBurke added the enhancement New feature or request label Feb 23, 2021

AndrewMBurke self-assigned this Feb 23, 2021

AndrewMBurke added 2 commits February 23, 2021 12:46

Bump minimum Auto Service version to rc2 to fix a Windows build issue

ed9c7d4

Fix quotes in examples build file

409c2fa

AndrewMBurke requested review from jradcliff and nwbirnie February 23, 2021 19:04

nwbirnie reviewed Feb 23, 2021

View reviewed changes

jradcliff reviewed Feb 23, 2021

View reviewed changes

Comment thread buildSrc/build.gradle

nwbirnie reviewed Feb 24, 2021

View reviewed changes

AndrewMBurke added 6 commits February 24, 2021 09:43

Removed explicit gax dependency for annotation processor

31bea74

Enable Gradle parallel build by default

01bb8b7

Set explicit target compatability to Java 1.8

5bd6771

Reformatted google-ads-examples/build.gradle

941ec47

Restructure task code, add runMigrationExample task

a351591

Remove extraneous comments

076950c

nwbirnie approved these changes Feb 25, 2021

View reviewed changes

nwbirnie reviewed Feb 25, 2021

View reviewed changes

Added build artifacts to .gitignore, fixed extraneous whitespace

d39e24d

AndrewMBurke requested a review from jradcliff February 26, 2021 17:40

jradcliff approved these changes Mar 18, 2021

View reviewed changes

AndrewMBurke merged commit 71816b9 into master Mar 18, 2021

AndrewMBurke deleted the add_gradle_build branch March 18, 2021 15:23


		description = 'Google Ads API client library for Java examples'

		public class ExampleRunnerTask extends JavaExec {

Conversation

AndrewMBurke commented Feb 23, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nwbirnie Feb 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

nwbirnie left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nwbirnie commented Mar 12, 2021

Uh oh!

jradcliff left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

nwbirnie Feb 24, 2021 •

edited

Loading