Gradle plugin for JVM projects written in Java, Kotlin, Groovy, or Scala; and Android projects written in Java or Kotlin. Provides advice for managing dependencies and other applied plugins
Apache License 2.0
1.67k
stars
115
forks
source link
Do not suggest to move dependencies between feature variants #1137
...as this may lead to confusing reports. I had the case where I got an advice like this:
These transitive dependencies should be declared directly:
testFixturesImplementation project(':producer')
Existing dependencies which should be modified to be as indicated:
extraImplementation project(':producer') (was testFixturesImplementation)
This suggests to first move a dependency away from 'testFixturesImplementation' to 'extraImplementation' and afterwards add it back to 'testFixturesImplementation'.
Instead, the advice should have been:
These transitive dependencies should be declared directly:
extraImplementation project(':producer')
The issue, as it seems, is the 'else' branch in the 'computeAdvice()' function. This is, if I understand correctly, for moving things that are already on the right scope, but in the wrong "sub-variant" (for lack of a better term) which makes sense for Android build types or flavors (e.g. 'api' vs 'debugApi') or in the special handling of 'test' ('test' vs 'testImplementation').
However, that code now interpreted different feature variants in one project similarly, leading to this weird overall advice: As two feature variants are not connected, moving something away from one of the variants means that you have to add it back afterwards.
The fix is rather crude, but I have to admit that this has grown so complex with the different cases that need to be supported that I would rather keep the change minimal to not break anything else.
...as this may lead to confusing reports. I had the case where I got an advice like this:
This suggests to first move a dependency away from 'testFixturesImplementation' to 'extraImplementation' and afterwards add it back to 'testFixturesImplementation'.
Instead, the advice should have been:
The issue, as it seems, is the 'else' branch in the 'computeAdvice()' function. This is, if I understand correctly, for moving things that are already on the right scope, but in the wrong "sub-variant" (for lack of a better term) which makes sense for Android build types or flavors (e.g. 'api' vs 'debugApi') or in the special handling of 'test' ('test' vs 'testImplementation').
However, that code now interpreted different feature variants in one project similarly, leading to this weird overall advice: As two feature variants are not connected, moving something away from one of the variants means that you have to add it back afterwards.
The fix is rather crude, but I have to admit that this has grown so complex with the different cases that need to be supported that I would rather keep the change minimal to not break anything else.