-
Notifications
You must be signed in to change notification settings - Fork 29
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
Gradle 6.2 as minimum #251
Conversation
To be honest, I'm not sure about that chance. I'm ambivalent to it. What do you propose? |
src/main/kotlin/io/github/gradlenexus/publishplugin/NexusPublishPlugin.kt
Show resolved
Hide resolved
src/main/kotlin/io/github/gradlenexus/publishplugin/NexusPublishPlugin.kt
Outdated
Show resolved
Hide resolved
@@ -54,8 +53,8 @@ class NexusPublishPlugin : Plugin<Project> { | |||
"Plugin must be applied to the root project but was applied to ${project.path}" | |||
} | |||
|
|||
require(GradleVersion.current() >= GradleVersion.version("6.0")) { | |||
"The plugin requires Gradle version 6.0+" | |||
require(GradleVersion.current() >= GradleVersion.version("6.2")) { |
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 was rather writing about benefits of bumping required version to 6.2 :-)
If only we had some stats of Gradle version distributions out in the wild, like Android does: https://www.androidauthority.com/wp-content/uploads/2023/06/Android-version-distribution-statistics-May-2023-988w-675h.jpg.webp
It would be a much easier to say "minimum x".
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 asked about that one friendly Gradle engineer ;-). We will see.
I feel convinced. Bumping from 6.0 to 6.2 should be easy for the majority of projects. Could you resolve the conflicts? |
Will do later. |
Co-authored-by: Róbert Papp <papp.robert.s@gmail.com>
I'm afraid, we hit a regression in e2e tests related to the properties handling:
However, as we do not check authorization (I've just created #256 as possible better regression testing passing credentials to Nexus in general), I'm not sure, if it is a problem with "gradleProperty" or we stopped passing credentials to Nexus at all (rather not). I plan to debug the e2e tests tomorrow to find out. |
Could the regression be this one from another PR? |
Rather not, e2e tests passed after that commit: https://github.com/gradle-nexus/publish-plugin/actions/runs/5561323077 It started failing after this MR was merged: |
Btw, just in case you would like to test some "risky" change with the e2e tests, it is enough to have the |
You can also run the workflow manually for any branch in that repo, using the button on the right: (I don't know, if it is possible to configure it to execute manually also for - safe - pull requests) |
* Bump minimum version to Gradle 6.1 * Use build service all the way (Gradle 6.1) * Bump minimum version to Gradle 6.2 * Use providers.gradleProperty (Gradle 6.2) * forUseAtConfigurationTime compatibility * Use full plugin name in error message Co-authored-by: Róbert Papp <papp.robert.s@gmail.com> * Revert "Use providers.gradleProperty (Gradle 6.2)" to not run into #267 This reverts commit f658356 --------- Co-authored-by: Marcin Zajączkowski <148013+szpak@users.noreply.github.com>
As promised, based on #242 (comment)
Doesn't give too many improvements, it actually looks more complicated 😅.