The Nebula lint plugin is a genuinely useful one. Though I agree that we should not be held back by it, we should at least understand what we are losing by not using it (temporarily). As far as I can tell, it prevent us from adding unnecessary or missing direct dependencies in Gradle. What else does it do?

Reviewed 26 of 26 files at r1, all commit messages.
Reviewable status: all files reviewed, 10 unresolved discussions (waiting on @gbrodman)

build.gradle line 41 at r1 (raw file):

  // Java static analysis plugins. Keep versions consistent with
  // ./buildSrc/build.gradle
  id 'nebula.lint' version '16.0.2'

Can we comment it out with a TODO to re-enable it once the incompatibility is resolved.

build.gradle line 112 at r1 (raw file):

// Provide defaults for all of the project properties.

// Only do linting if the build is successful.

Ditto.

buildSrc/build.gradle.kts line 35 at r1 (raw file):

plugins {
  // Java static analysis plugins. Keep versions consistent with ../build.gradle
  // id("nebula.lint") version "16.0.2"  // unsupported for kotlin

It seems like Nebula has no plans to support kotlin for the foreseeable future: nebula-plugins/gradle-lint-plugin#166

I'd leave the comment here and cite the issue above. Also it seems pretty clear that Nebula is just a "side" project for Netflix, where we export whatever they are working on internally. This means that the community's need is probably not their top concern. Maybe we should re-considering a more open-minded linter (if one exists).

buildSrc/build.gradle.kts line 87 at r1 (raw file):

}

// checkstyle {

Not specific to this PR, but do we know why this part was commented out? Are we not running checkstyle?

buildSrc/build.gradle.kts line 92 at r1 (raw file):

dependencies {
  val deps = project.ext["dependencyMap"] as Map<String, String>

Why change it to .?

buildSrc/build.gradle.kts line 122 at r1 (raw file):

tasks.register("exportDependencies") {
  val outputFileProperty = "dependencyExportFile"

Why is this not needed any more?

core/build.gradle line 152 at r1 (raw file):

// - The (test/)compile/runtime labels are deprecated. We continue using these
//   labels due to nebula-lint.
//   TODO(weiminyu): switch to api/implementation labels.

This comment can be removed. We switched to api/implementation labels already.

core/build.gradle line 206 at r1 (raw file):

  implementation deps['com.google.guava:guava']
  implementation deps['com.google.protobuf:protobuf-java']
  gradleLint.ignore('unused-dependency') {

We might want to keep this part but comment it out (and also leave the comment about regarding gradleLint.ignore, in case we want to turn the linter back on in the future.

gradle/wrapper/gradle-wrapper.properties line 3 at r1 (raw file):

distributionBase=GRADLE_USER_HOME
distributionPath=wrapper/dists
<<<<<<< Updated upstream

booooooo

integration/build.gradle line 42 at r1 (raw file):

dependencies {
  gradleLint.ignore('unused-dependency') {

Same here, comment it out but don't delete it.

I'm not sure if we've found any use from that other than identifying unused dependencies, but it's very possible that I'm missing something.

Reviewable status: 21 of 26 files reviewed, 10 unresolved discussions (waiting on @jianglai)

build.gradle line 41 at r1 (raw file):

Done.

build.gradle line 112 at r1 (raw file):

Done.

buildSrc/build.gradle.kts line 35 at r1 (raw file):

Yep, seems that way.

buildSrc/build.gradle.kts line 87 at r1 (raw file):

there's a duplicate checkstyle block elsewhere

buildSrc/build.gradle.kts line 92 at r1 (raw file):

Gradle complained about a type inference warning. It shouldn't matter in reality.

buildSrc/build.gradle.kts line 122 at r1 (raw file):

The removed variables are unused

core/build.gradle line 152 at r1 (raw file):

Done.

core/build.gradle line 206 at r1 (raw file):

Done.

gradle/wrapper/gradle-wrapper.properties line 3 at r1 (raw file):

lol wtf i don't remember it notifying me

integration/build.gradle line 42 at r1 (raw file):

Done.

Let's at least include that in the PR description, so we know what we are giving up at a glance.

Reviewed 5 of 5 files at r2, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @gbrodman)