Cleanup Gradle dependencies and miscellaneous improvements #1856
Conversation
- Inline versions that were used only once, allowing us to use the compact form for libraries & plugins - Merge TOML declarations with `group` + `name` into the unique `module` component - Expose new BoMs to enforce versioning across packages (`coil-kt-bom`, `kotlin-bom`, `kotlinx-coroutines-bom`, `kotlinx-serialization-bom`, `protobuf-bom`, `okhttp-bom`, `retrofit-bom`) - Refactor `jacoco` version declaration into a proper dependency declaration to enable actual dependency tracking - Rename `androidx.hilt.navigation.compose` to `hilt.ext.navigation.compose` to match the already existing `hiltExt` group. - Replace duplicated `*.gradlePlugin` dependencies with converter that converts plugin alias to usable dependencies. - Sort dependencies - Remove unused `kotlinx-coroutines-android` dependency
- Use BOM platform dependency. - Remove -kt suffix from dependency name has they serve no real purpose. - Add `coil-network` artifact to configure `CallFactory`. - Although "First party Decoder" `SvgDecoder` is now automatically added to each new ImageLoader through a service loader, keeping it in code prevents us from removing the dependency that would appear unused otherwise. - Replace explicit `respectCacheHeaders(false)` with `CacheStrategy::DEFAULT` which does not respect cache-control headers. - Migrate `ImageLoaderFactory` to `SingletonImageLoader.Factory` - Inline TOML dependencies (see android#1856) docs: https://coil-kt.github.io/coil/upgrading_to_coil3/
| @@ -1,166 +1,118 @@ | |||
| [versions] | |||
| accompanist = "0.37.0" | |||
There was a problem hiding this comment.
I'm not a fan of inlining versions that only appear once - it does remove lines from this file, but I find it a lot clearer to have all of the versions in continuous place.
For future maintenance, this also seems like this would be more likely to have a dependency get copied in the same version group without moving that shared version back to the versions section.
There was a problem hiding this comment.
I'd love to agree with you, but in practice, this almost never holds true 😛.
Having top level [versions] should be the exception, not the norm.
Let me try to explain my reasoning:
I find it a lot clearer to have all of the versions in continuous place.
I kind of disagree on this, but this is a personal opinion. I can't trust top-level variables like this, as I don't know exactly what they are used for, unless I also check where (and if) the variable is actually used/referenced in the file. So it always end up in extra checks and lookups everytime.
I always prefer simplicity, meaning less indirections.
For future maintenance, this also seems like this would be more likely to have a dependency get copied in the same version group without moving that shared version back to the versions section.
Again, in practice this is almost never what actually happens. We almost always update dependencies with automated tools like renovate, dependabot or whatever. And they take care of this for us. There is also Android Lint rules for this, when multiple versions are referenced for the same dependency. (I also implemented custom lint rules to avoid cases like this)
And most of the multi-artifacts dependencies are now slowly migrating to BoMs making the overall process simpler and less error prone, like Jetpack Compose (except all the other Jetpack library groups for whatever reason 😡)
Anyways, I'm not here to impose anything, but only to suggest improvements I feel worthy, so let me know if I should proceed to the un-inlining of versions in this file 😉
group+nameinto the uniquemodulecomponentcoil-kt-bom,kotlin-bom,kotlinx-coroutines-bom,kotlinx-serialization-bom,protobuf-bom,okhttp-bom,retrofit-bom)jacocoversion declaration into a proper dependency declaration to enable actual dependency trackingandroidx.hilt.navigation.composetohilt.ext.navigation.composeto match the already existinghiltExtgroup.*.gradlePlugindependencies with converter that converts plugin alias to usable dependencies.kotlinx-coroutines-androiddependency