fix gradle dependencies order code smell #9079
Conversation
| @@ -24,6 +24,7 @@ ext { | |||
|
|
|||
| dependencies { | |||
| // jhipster-needle-gradle-dependencies | |||
There was a problem hiding this comment.
If we want to be consistent, I guess that it should be:
| // jhipster-needle-gradle-dependencies | |
| // jhipster-needle-gradle-implementation-dependencies |
| return switch (gradleScope) { | ||
| case GradleDependencyScope.TEST_IMPLEMENTATION -> GRADLE_TEST_DEPENDENCY_NEEDLE; | ||
| case GradleDependencyScope.RUNTIME_ONLY -> GRADLE_RUNTIME_DEPENDENCY_NEEDLE; | ||
| default -> GRADLE_DEPENDENCY_NEEDLE; |
There was a problem hiding this comment.
I suggest to remove TEST_COMPILE_ONLY and TEST_RUNTIME_ONLY values from GradleDependencyScope, since they're currently not used.
And remove this default branch: it's better to fully use pattern matching so each new value will have to be explicitly mapped.
This is related to my comment #9078 (comment): we'll need additional needles for compileOnly and runtimeOnly
There was a problem hiding this comment.
NB : GradleDependencyScope.API should also be removed :)
| @@ -97,7 +101,7 @@ private void addDependencyToBuildGradle(JavaDependency dependency) { | |||
| MandatoryReplacer replacer = new MandatoryReplacer( | |||
| new RegexNeedleBeforeReplacer( | |||
| (contentBeforeReplacement, newText) -> !contentBeforeReplacement.contains(newText), | |||
| gradleScope == GradleDependencyScope.TEST_IMPLEMENTATION ? GRADLE_TEST_DEPENDENCY_NEEDLE : GRADLE_DEPENDENCY_NEEDLE | |||
| determineGradleDependencyNeedle(gradleScope) | |||
There was a problem hiding this comment.
Naming suggestion:
| determineGradleDependencyNeedle(gradleScope) | |
| needleForGradleDependencyScope(gradleScope) |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9079 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 2903 2906 +3
===========================================
Files 733 733
Lines 12527 12531 +4
Branches 253 252 -1
===========================================
+ Hits 12527 12531 +4 ☔ View full report in Codecov by Sentry. |
renanfranca
force-pushed
the
9078-gradle-dependencies-order-code-smell
branch
from
b9271b6
to
c039596
Compare
|
@murdos : Thank you for the fast review! I implemented the bearer minimum to solve the particular reported code smell, when another scenario of dependencies code smells appears I am going to fix it. I tried dependencies {
implementation(libs.protobuf.java)
implementation(libs.commons.lang3)
implementation(platform(libs.spring.boot.dependencies))
implementation(libs.spring.boot.starter)
implementation(libs.spring.boot.configuration.processor)
implementation(libs.spring.boot.starter.validation)
implementation(libs.spring.boot.starter.web)
implementation(libs.spring.boot.starter.actuator)
implementation(libs.spring.boot.starter.data.jpa)
implementation(libs.hikariCP)
implementation(libs.hibernate.core)
implementation(libs.liquibase.core)
implementation(libs.spring.boot.starter.security)
implementation(libs.jjwt.api)
implementation(libs.springdoc.openapi.starter.webmvc.ui)
implementation(libs.springdoc.openapi.starter.webmvc.api)
// jhipster-needle-gradle-implementation-dependencies
runtimeOnly(libs.postgresql)
runtimeOnly(libs.jjwt.impl)
runtimeOnly(libs.jjwt.jackson)
// jhipster-needle-gradle-runtime-dependencies
testImplementation(libs.protobuf.java.util)
testImplementation(libs.spring.boot.starter.test)
testImplementation(libs.reflections)
testImplementation(libs.testcontainers.postgresql)
testImplementation(libs.h2)
testImplementation(libs.spring.security.test)
testImplementation(libs.cucumber.junit.platform.engine)
testImplementation(libs.cucumber.java)
testImplementation(libs.cucumber.spring)
testImplementation(libs.junit.platform.suite)
// jhipster-needle-gradle-test-dependencies
} |
| @@ -118,7 +130,6 @@ private static String applyVersionCatalogReferenceConvention(String rawVersionCa | |||
| private static GradleDependencyScope gradleDependencyScope(JavaDependency dependency) { | |||
| return switch (dependency.scope()) { | |||
| case TEST -> GradleDependencyScope.TEST_IMPLEMENTATION; | |||
| case PROVIDED -> GradleDependencyScope.COMPILE_ONLY; | |||
There was a problem hiding this comment.
Sorry, but I would like to keep this one.
Even if the PROVIDED scope is not used yet in any modules in jhlite, it might be used in some custom jhlite, and the mapping is correct, I don't see a reason to remove it.
There was a problem hiding this comment.
No problem, I will keep it 👍. In that case, should I create a needle for compileOnly?
There was a problem hiding this comment.
Yes, I think it's necessary. Please let me know if you don't have time to do it, I'll do it.
There was a problem hiding this comment.
Yes, I think it's necessary. Please let me know if you don't have time to do it, I'll do it.
Thank you 😊, I can't do it today, only tomorrow morning. You can do it if you want 👍
renanfranca
force-pushed
the
9078-gradle-dependencies-order-code-smell
branch
from
c039596
to
4a4d06c
Compare
|
@murdos : I pushed the changes, please, let me know if it's ok 😁 |
|
@renanfranca : it's perfect, thanks! Sorry I finally wasn't able to work on it yesterday as I initially proposed. |
It's cool, let's keep going! 😎 |
Fix #9078