-
Notifications
You must be signed in to change notification settings - Fork 493
Adds gradle-dependency-graph-generator-plugin to the project #671
Conversation
MapboxAndroidDemo/build.gradle
Outdated
@@ -2,6 +2,7 @@ apply plugin: 'com.android.application' | |||
apply plugin: 'com.google.firebase.firebase-perf' | |||
apply plugin: 'com.github.triplet.play' | |||
apply from: "$project.rootDir/gradle/script-git-version.gradle" | |||
apply from: "$project.rootDir/gradle/graph.gradle" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to understand in a glance what the plugin does, what about calling it dependencies-graph.gradle
instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Renamed to gradle-dependencies-graph.gradle
. Looks like the Maps SDK likes prefixing all scripts with gradle-
.
gradle/graph.gradle
Outdated
def mapboxGenerator = new Generator( | ||
"mapboxLibraries", // Suffix for our Gradle task. | ||
"", // Root suffix that we don't want in this case. | ||
{ dependency -> dependency.getModuleGroup().startsWith("com.mapbox") }, // Only want Mapbox libs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Our GROUP
is com.mapbox.mapboxsdk
. Shouldn't we use the complete group here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed, although it doesn't have an effect in the check.
gradle/graph.gradle
Outdated
} | ||
|
||
dependencies { | ||
classpath "com.vanniktech:gradle-dependency-graph-generator-plugin:0.3.0" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For consistency we should add this dependency in the dependencies.gradle file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the same approach script-git-version.gradle
follows. If we're gonna change one script for consistency, we should change them all - feel free to cut a separate ticket.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Follow up ticket 👉 #676
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add integration inside the makefile?
@Guardiola31337 @tobrun Ready for another round of 👀 . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After getting ✅ from CI,
…oject (#11603) * adds gradle-dependency-graph-generator-plugin to the project * keep script in sync with mapbox/mapbox-android-demo#671 * adds a makefile rule for dependency generation
This helps us visualize the dependency tree for Mapbox dependencies. To generate the chart, run the following Gradle task:
Graphviz needs to be installed following the instructions on the plugin README.
cc: @langsmith @Guardiola31337