-
Notifications
You must be signed in to change notification settings - Fork 220
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
Replace taskGraph.afterTask with an operation completion listener in Gradle 6.1+ #389
Conversation
Just to let you know, the plugin will not function on
|
You mean the plugin will not function with 8.x if you enable configuration cache, which is not different in 8.7 from earlier versions. |
try { | ||
createScmAdapter() | ||
} catch (Exception ignored) {} | ||
if (scmAdapter && extension.revertOnFail && project.file(extension.versionPropertyFile)?.exists()) { |
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.
if (scmAdapter && extension.revertOnFail && project.file(extension.versionPropertyFile)?.exists()) { | |
if (scmAdapter && extension.revertOnFail.get() && project.file(extension.versionPropertyFile)?.exists()) { |
Not sure what groovy does if we don't use the get() .
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.
The same it does currently, I just moved this existing code to a method.
But you are fully right of course, for Groovy it will just always be true
as it is non-null.
This is already like that since switching to properties and not adapting this usage (and maybe others?)
One of the fun things you have because of using Groovy for the plugin. :-D
I now fixed it together with this PR.
|
||
abstract class ReleasePlugin extends PluginHelper implements Plugin<Project> { |
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.
What is the reason for this change?
I see that you added
@Inject
abstract protected ObjectFactory getObjects();
@Inject
abstract protected ProviderFactory getProviders();
But I don't get the why.
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.
Because I use them at
objects.newInstance(BuildEventsListenerRegistryProvider) |
.onTaskCompletion(providers.provider { |
Of course you can get those from project
too, but that is bad / outdated practice and I didn't want to add to the existing ones but doing it right. :-)
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 see. Thanks for the explanation 👍
7ffe081
to
21d5cc0
Compare
No description provided.