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
Fix Gradle deprecations #3892
Fix Gradle deprecations #3892
Conversation
6070f10
to
77942a5
Compare
SonarCloud Quality Gate failed. |
53177db
to
7223718
Compare
SonarCloud Quality Gate failed. |
@Sineaggi Apologies for the number of PRs that fell off our radar - we will take some time to review them and appreciate your patience! |
@emmileaf Take your time, and thanks for reaching out! |
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.
@Sineaggi Thanks for helping to address these deprecation warnings! I’ve left two quick inline comments below.
@@ -1,5 +1,5 @@ | |||
distributionBase=GRADLE_USER_HOME | |||
distributionPath=wrapper/dists | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-6.9.2-bin.zip | |||
distributionUrl=https\://services.gradle.org/distributions/gradle-6.9.3-bin.zip |
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.
Are we able to exclude these gradle wrapper updates and keep the existing version, or is it tied to another aspect of this PR that I am missing?
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.
Nope, it's just bumping gradle to the latest patch version is one of the first things I do when working on deprecation warnings.
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.
Gotcha. If it's not a blocker, could you exclude the files related to gradle build version upgrade, and keep this PR just for the deprecation-related fixes? Thanks!
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.
Excluded
build.gradle
Outdated
} | ||
} | ||
|
||
configurations { | ||
integrationTestImplementation.extendsFrom testImplementation | ||
integrationTestImplementation.setCanBeResolved(true) |
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.
Ah, I’m curious to what IDE you are using to develop with? Digging back in history a bit (#2961, #2375), I think some of this integrationTestImplementation
setup may have been added to work around some Eclipse-specific issues, but I don’t personally use Eclipse for development and don’t have a quick way to confirm. It would be great to know if they no longer causing problems though!
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.
I use Intellij, which didn't need this. If it's required for Eclipse, keeping it seems reasonable.
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.
Let's keep it in if it doesn't hurt.
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.
Sounds good
7223718
to
dfc477e
Compare
SonarCloud Quality Gate failed. |
SonarCloud Quality Gate failed. 0 Bugs 65.0% Coverage Catch issues before they fail your Quality Gate with our IDE extension SonarLint |
Thank you for your contribution @Sineaggi! |
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com>
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com> # Conflicts: # build.gradle # jib-cli/build.gradle # jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/GradleProjectProperties.java # jib-gradle-plugin/src/main/java/com/google/cloud/tools/jib/gradle/JibPlugin.java # jib-gradle-plugin/src/test/java/com/google/cloud/tools/jib/gradle/GradleProjectPropertiesTest.java
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com>
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com>
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com>
Co-authored-by: Clayton Walker <cwalker@sofi.org> Co-authored-by: Emily Wang <emmwang@google.com>
JavaPluginConvention
in favor ofJavaPluginExtension
com.google.cloud.tools.jib.properties
is already being generated by the gradle plugin