Skip to content

Kotlin Gradle scripts. Part 1 of 2#532

Merged
dmdashenkov merged 32 commits intomasterfrom
kotlin-gradle-scripts
May 18, 2020
Merged

Kotlin Gradle scripts. Part 1 of 2#532
dmdashenkov merged 32 commits intomasterfrom
kotlin-gradle-scripts

Conversation

@dmdashenkov
Copy link
Contributor

@dmdashenkov dmdashenkov commented May 8, 2020

In this PR we start migrating our Gradle scripts to Kotlin.

For simplicity, we split the changes into 2 PRs. In this PR we migrate the basic scripts of the repo's main build. Other builds (smoke-tests and base-validating-builders) are to be migrated in the next part.

@dmdashenkov dmdashenkov self-assigned this May 8, 2020
@codecov
Copy link

codecov bot commented May 8, 2020

Codecov Report

Merging #532 into master will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master     #532   +/-   ##
=========================================
  Coverage     73.70%   73.70%           
  Complexity     2930     2930           
=========================================
  Files           503      503           
  Lines         11873    11873           
  Branches        665      665           
=========================================
  Hits           8751     8751           
  Misses         2902     2902           
  Partials        220      220           

@dmdashenkov dmdashenkov changed the title Kotlin Gradle scripts Kotlin Gradle scripts. Part 1 of 2 May 18, 2020
@dmdashenkov dmdashenkov marked this pull request as ready for review May 18, 2020 10:25
@dmdashenkov dmdashenkov requested a review from armiol May 18, 2020 10:25
@dmdashenkov
Copy link
Contributor Author

@armiol, @alexander-yevsyukov, PTAL.

Copy link
Collaborator

@armiol armiol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmdashenkov mostly LGMT. Please see my comments.

)
@Deprecated("Use Flogger over SLF4J.",
replaceWith = ReplaceWith("Deps.runtime.floggerSystemBackend"))
@Suppress("DEPRECATION")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this suppression. Can you please have a comment?

testImplementation deps.build.errorProneTestHelpers
annotationProcessor(Deps.build.autoService.processor)
compileOnly(Deps.build.autoService.annotations)
implementation(project(":base"))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one reads better than "implementation" in build.gradle.kt above, i.e.

        "implementation"(Deps.build.guava)
        "implementation"(Deps.build.checkerAnnotations)
        "implementation"(Deps.build.jsr305Annotations)

It is possible to use no-quote syntax there as well?

sourceSets.test {
resources.srcDir("$projectDir/src/test/resources")
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please kill these redundant empty lines.

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see my comments and questions.

.travis.yml Outdated
# Encrypted `GCS_SECRET` variable.
- secure: "i8MhONZu7QjyM2V887A1Tydr1WMqQP5jJZNjIJjc1Uae8F0/z8cJZIZ1hstodN7FpoR4VF92zyhUwbt6fz/dsdPEJFccsiMlEc9vlqecQCd267160wgRZneaB6Xe/y/EUmq9XsGdn/k1Ey+QZwX9au/8RU191v+fDsCtMRYXzyEa/BvbQuSwuYRgQDxTAxuJgTmG5Sxl9jWqKw1BfxUcEoErc/jqymU58w6z2TxKxVzIXT29Jy/Z12VuSiS8opigSrIP8e/1fctC84wI7S52mext2ZfhPYSTHFKS+xg1vQDYPb8m5aomL8E6Of7hVD5BTnEnyjj+/Gr63GAzHXtkHhWoxo+vB+xBFfDu8wxM5Aqna3H7LMDD5kGCxQEz8qmzHBHMAhLnhsRzjNVu2+tLCZdeMN88Ud2uemL2SCAcR8Juleg7DGMj3D0SAbPyUH3+9yYYWzSg6iaxgTdHBnJ+uXUJp0Nu+M2EK6Kl+pYAsCLVfZRPGaajFXVnJEPPeSr2PYzk7F4pIzgn/E8AtYEJ0gcEbjoTItS8EjliJKDXM4HdkluXBFLvzIH1O1nCtxKNv4UkUmPhFbfHrPXDcsYq2zsEe+NkvsJlxjAwYnOMkT4NLiEsec1a7K9bBC+iQA9e8rriMbu6/1w63JErQyx05avPjgO8XRDK8hxTf4rhBmY="
- GRADLE_OPTS="-Xmx2G"
# TODO:2020-05-18:dmytro.dashenkov: Remove there 2 lines when Travis stops failing when installing JDK 8.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there -> these?


configurations {
// Avoid collisions of Java classes defined both in `protobuf-lite` and `protobuf-java`
runtimeClasspath.get().exclude(group = "com.google.protobuf", module = "protobuf-lite")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we have these constants extracted into final variables?

/**
* Checks if the given file belongs to the Google `.proto` sources.
*/
fun isGoogleProtoSource(file: FileTreeElement): Boolean {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible to do the following in this context:

fun FileTreeElement.isGoogleProtoSource(): Boolean { ... }

build.gradle.kts Outdated
from("$rootDir/config/gradle/dependencies.gradle")
}
version = rootProject.extra["versionToPublish"]!!
version = rootProject.extra["versionToPublish"]!!
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need this twice?

build.gradle.kts Outdated
Deps.build.protobuf.forEach { "api"(it) }
"api"(Deps.build.flogger)
"implementation"(Deps.build.guava)
"implementation"(Deps.build.checkerAnnotations)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there calls that allow to say implementation only once, accepting a list of dependencies?

build.gradle.kts Outdated

DependencyResolution.forceConfiguration(configurations)
configurations {
named("runtime").get().exclude(group = "com.google.protobuf", module = "protobuf-lite")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this becomes a “reusable code”. Can we extract it somehow?

Copy link
Contributor

@alexander-yevsyukov alexander-yevsyukov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@dmdashenkov dmdashenkov merged commit 2d5b632 into master May 18, 2020
@dmdashenkov dmdashenkov deleted the kotlin-gradle-scripts branch May 18, 2020 14:31
@dmitrykuzmin dmitrykuzmin mentioned this pull request Sep 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants