-
Notifications
You must be signed in to change notification settings - Fork 373
fix(gradle-plugin): replace usages of deprecated methods #11399
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(gradle-plugin): replace usages of deprecated methods #11399
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11399 +/- ##
=========================================
Coverage 57.78% 57.78%
Complexity 1711 1711
=========================================
Files 347 347
Lines 12904 12904
Branches 1238 1238
=========================================
Hits 7456 7456
Misses 5000 5000
Partials 448 448
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
250f963 to
cc758f2
Compare
|
Can somebody please approve the workflows? I believe I fixed the test failures. |
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsTest.kt
Fixed
Show fixed
Hide fixed
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsTest.kt
Fixed
Show fixed
Hide fixed
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsTest.kt
Fixed
Show fixed
Hide fixed
|
|
||
| class GradleVersionsTest : FunSpec({ | ||
| withData( | ||
| null, // 7.x, from the project's gradle/gradle-wrapper.properties |
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.
Somewhat related: I believe we haven't formally decided about the oldest Gradle version we'd want to support with GradleInspector. It more was like a best-effort approach so far. Looking at this, should we now formalize that we only support Gradle 7 and onward?
If we decide to go even further down, like Gradle 6.7 (first version with toolchain support) or even 5.0 (first version with Kotlin DSL support), we should add these version here. But I'd do so only if it doesn't cost us much in terms of code changes right now.
Options, @oss-review-toolkit/core-devs?
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.
about the oldest Gradle version we'd want to support with
GradleInspector
Oh, I just realized this PR is about the Gradle analyzer, not the GradleInspector one.
@sergej-koscejev, is there a specific reason why you are using the legacy Gradle analyzer instead of GradleInspector?
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.
Hmm, I think I got confused and thought that this code is used by both analyzers. I have re-checked now on my Gradle 9 project and realized my mistake, it seems to work with gradle-inspector so far.
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 any case, I believe your changes are good for anyone who's still stuck with the Gradle analyzer, so I think we should merge them once there are no more findings.
cc758f2 to
ae0bce5
Compare
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsFunTest.kt
Show resolved
Hide resolved
This comment was marked as resolved.
This comment was marked as resolved.
ae0bce5 to
1d5d098
Compare
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsTest.kt
Outdated
Show resolved
Hide resolved
Add a test for multiple Gradle versions. Signed-off-by: Sergej Koščejev <[email protected]>
1d5d098 to
04818fc
Compare
plugins/package-managers/gradle/src/funTest/kotlin/GradleVersionsFunTest.kt
Dismissed
Show dismissed
Hide dismissed
|
|
This confused me because with Gradle version |
It seems so, yes, although even then I currently don't understand why this happens, as all Gradle tests I've checked explicitly use Java 17, but class file version 65 belongs to Java 21. Anyway, let me work on some improvements to isolate things, and me take over your PR; I'll make some minor changes and rebase on top of those upcoming changes. |
Some new Gradle version related tests will come as part of [1]. [1]: #11399 Signed-off-by: Sebastian Schuberth <[email protected]>
Some new Gradle version related tests will come as part of [1]. [1]: #11399 Signed-off-by: Sebastian Schuberth <[email protected]>
Some new Gradle version related tests will come as part of [1]. [1]: #11399 Signed-off-by: Sebastian Schuberth <[email protected]>
LenientConfiguration#getArtifacts(Spec)andBuildIdentifier#isCurrentBuild()were deprecated in Gradle 8 and removed in Gradle 9. Rewrite their usages to support Gradle 9.