Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix all Gradle build warnings #1170

Open
wants to merge 8 commits into
base: master
Choose a base branch
from
Open

Fix all Gradle build warnings #1170

wants to merge 8 commits into from

Conversation

TWiStErRob
Copy link
Contributor

@TWiStErRob TWiStErRob commented Jan 3, 2018 •
edited
Loading

Initially re #1026 (comment), but then I fixed some (almost all) Gradle build warnings (full unabridged output sans ProGuard; daren't put the before log in the PR text, so attached 😉):

$gradlew clean assemble check connectedCheck --warning-mode=all

> Configure project :butterknife
The CompileOptions.bootClasspath property has been deprecated and is scheduled to be removed in Gradle 5.0. Please use the CompileOptions.bootstrapClasspath property instead.

> Task :butterknife:lint
Gradle now uses separate output directories for each JVM language, but this build assumes a single directory for all classes from a source set. This behaviour has been deprecated and is scheduled to be removed in Gradle 5.0
Ran lint on variant release: 0 issues found
Ran lint on variant debug: 0 issues found
No issues found.

> Task :butterknife-integration-test:lint
Ran lint on variant debug: 0 issues found
Ran lint on variant release: 0 issues found
No issues found.

> Task :butterknife:connectedDebugAndroidTest
Starting 32 tests on Nexus_5_API_19(AVD) - 4.4.2

> Task :butterknife-integration-test:connectedDebugAndroidTest
Starting 1 tests on Nexus_5_API_19(AVD) - 4.4.2


BUILD SUCCESSFUL in 2m 0s
212 actionable tasks: 210 executed, 2 up-to-date

(both of the remaning warnings are because of Gradle plugin 3.0.1 + Gradle 4.4.1 combination, will disappear once 3.1.0 is used or downgrade to Gradle 4.2.1)

More details:

  • :butterknife:androidJavadocs:
    • add compile classpath to JavaDoc classpath to rid of 236 warns and 95 errors
      (327 related to support-annotations, 2 to appcompat, and 2 to butterknife-annotations)
    • this implies that we need to build butterknife-annotations first, hence dependsOn javaPreCompileRelease which jars dependent subprojects.
    • DebouncingOnClickListener: FQCN to get rid of 3 additional JavaDoc warnings
    • show more warns and errors and increase memory (it failed with default 32M for me when listing the warns/errors)
    • potentially Xdoclint:none can be removed now
  • ButterKnifePlugin.kt: warn at compile time:

    Unnecessary non-null assertion (!!) on a non-null receiver of type T!

  • resolutionStrategy.force deps.kotlin.reflect to fix:

    w: Runtime JAR files in the classpath should have the same version.
    w: Consider providing an explicit dependency on kotlin-reflect 1.2 to prevent strange errors
    w: Some runtime JAR files in the classpath have an incompatible version. Consider removing them from the classpath or use '-Xskip-runtime-version-check' to suppress this warning

  • -Xlint:-options hides

    warning: [options] bootstrap class path not set in conjunction with -source 1.7

  • -Xlint:all + Werror for strict compilation:
    • in AndroidTests I suppressed the deprecations, because I don't know if those should be ContextCompat.getBlah or not, the generated code definitely uses those methods.
    • SimpleActivityTest also just suppressed, because it feels like new runner + Espresso is a bit overkill there

@TWiStErRob
Copy link
Contributor Author

Rebased onto latest master including Gradle update and integration tests. Needed f6563e5 to be able to test.

@WOLVIE97
Copy link

How about this?

jaredsburrows
jaredsburrows reviewed Dec 20, 2018
View reviewed changes
build.gradle
@@ -74,7 +76,7 @@ subprojects { project ->
google()
}

if (!project.name.equals('butterknife-gradle-plugin')) {
if (project.name != 'butterknife-gradle-plugin') {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

jaredsburrows
jaredsburrows reviewed Dec 20, 2018
View reviewed changes
build.gradle
@@ -93,4 +95,26 @@ subprojects { project ->
}
}
}

project.tasks.withType(JavaCompile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

jaredsburrows
jaredsburrows reviewed Dec 20, 2018
View reviewed changes
sample/library/build.gradle
@@ -6,7 +6,7 @@ buildscript {
}

dependencies {
classpath "com.jakewharton:butterknife-gradle-plugin:${versions.release}"
classpath deps.release.gradlePlugin
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

jaredsburrows
jaredsburrows reviewed Dec 20, 2018
View reviewed changes
gradle/gradle-mvn-push.gradle
}
options {
jFlags '-Xmx128M'
addStringOption 'Xmaxwarns', '1000'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@TWiStErRob
Copy link
Contributor Author

Feels like I should rebase again :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jaredsburrows jaredsburrows jaredsburrows left review comments

@maherabdalhameb maherabdalhameb maherabdalhameb approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@TWiStErRob @WOLVIE97 @jaredsburrows @maherabdalhameb