Validating runtime dependencies for conflicts #518
Conversation
AnatolyPopov
force-pushed
the
anatolii/dependency-validation
branch
2 times, most recently
from
cd6a2b8
to
5e5325f
Compare
AnatolyPopov
force-pushed
the
anatolii/dependency-validation
branch
from
5e5325f
to
f27edbf
Compare
There was a problem hiding this comment.
@AnatolyPopov thanks for looking into this.
I like that it's more idiomatic now using gradle APIs to enforce the dependencies; but this also makes it harder to reason about if don't know the gradle internals. Hence I'd ask for some additional comments to clarify how to maintain this approach in the future.
About the "plugins" submodule, I'd say we may be overusing the term in other places (the project itself is sometimes referred as plugin). Could we consider backend instead as it's also known in the codebase?
What do you think about re-structuring the commits in a way that it's clear to see how enabling this new validation strategy flags conflicting dependencies? similar to https://github.com/Aiven-Open/tiered-storage-for-apache-kafka/pull/442/commits.
I think we are also missing cleaning up the previous validation scripts. Could we include it on this PR as well?
| @@ -74,6 +74,11 @@ subprojects { | |||
| withJavadocJar() | |||
| withSourcesJar() | |||
| } | |||
| configurations.runtimeClasspath { | |||
There was a problem hiding this comment.
Could we add some comments here to clarify how is affecting the deps validation? e.g. usage of platform feature, etc.
| @@ -123,29 +128,32 @@ subprojects { | |||
|
|
|||
| azureSdkVersion = "1.2.21" | |||
|
|
|||
| testcontainersVersion = "1.19.4" | |||
| testcontainersVersion = "1.19.7" | |||
There was a problem hiding this comment.
was this needed as part of this PR? if it's maybe a separate commit will help.
|
|
||
| testcontainersFakeGcsServerVersion = "0.2.0" | ||
| } | ||
|
|
||
| dependencies { | ||
| compileOnly "org.apache.kafka:kafka-clients:$kafkaVersion" | ||
| compileOnly "org.apache.kafka:kafka-storage-api:$kafkaVersion" | ||
| compileOnly("org.apache.kafka:kafka-storage-api:$kafkaVersion") |
There was a problem hiding this comment.
wondering if this is a style change only or it has some implications in the context of this PR?
|
|
||
| implementation enforcedPlatform("com.fasterxml.jackson:jackson-bom:$jacksonVersion") |
There was a problem hiding this comment.
could we add some comments on what this "enforcePlatform" does and why we needed it here?
e.g. as it's only added for jacksone, does it mean that we would need to know in advance which dependencies may conflict, so we can flag it as we are doing here?
| configurations { | ||
| providedRuntime | ||
| implementation.extendsFrom(providedRuntime) | ||
| } |
There was a problem hiding this comment.
maybe some comments here as well
No description provided.