Use domain object container to avoid afterEvaluate. #9
Conversation
| /** | ||
| * Configuration for [DependencyGuardPlugin] | ||
| */ | ||
| public class DependencyGuardConfiguration( | ||
| public open class DependencyGuardConfiguration @Inject constructor( |
There was a problem hiding this comment.
The changes here were necessary to
- Let me use
ObjectFactoryto instantiate this class - Let me add instances of this class to a domain object container.
| baselineTask: TaskProvider<DependencyGuardListTask>, | ||
| guardTask: TaskProvider<DependencyGuardListTask> | ||
| ) { | ||
| extension.container.all { |
There was a problem hiding this comment.
The all callback is the magic that makes it work. It triggers the action on each item in the container, and added to the container.
| val c = objects.newInstance(DependencyGuardConfiguration::class.java, name).apply { | ||
| config.execute(this) | ||
| } | ||
| container.add(c) |
There was a problem hiding this comment.
Can't use container.create(name, config) for reasons I indicated in slack.
autonomousapps
force-pushed
the
trobalik.domainobjects
branch
from
2121f83 to
0dc6296
Compare
| /** | ||
| * Whether to allow a dependency. | ||
| * | ||
| * TODO: not sure how to model this as a task input. May not matter since the task that uses it |
There was a problem hiding this comment.
Note this TODO.
| @@ -3,7 +3,7 @@ package com.dropbox.gradle.plugins.dependencyguard.internal | |||
| import com.dropbox.gradle.plugins.dependencyguard.DependencyGuardPlugin | |||
| import org.gradle.api.Project | |||
|
|
|||
| internal fun Project.isRootProject(): Boolean = (path == rootProject.path) | |||
| internal fun Project.isRootProject(): Boolean = this == rootProject | |||
There was a problem hiding this comment.
simpler check
|
|
||
| internal open class DependencyGuardListTask : DefaultTask() { | ||
| public abstract class DependencyGuardListTask : DefaultTask() { |
There was a problem hiding this comment.
All tasks should be abstract by default (preferable to open, as it allows abstract/managed property inputs, see below). And I don't see a point in marking a task as internal since Gradle extends it at runtime anyway, ignoring the visibility marker entirely.
There was a problem hiding this comment.
I think the apiCheck binary compatibility validator might care?
| @@ -56,9 +56,28 @@ internal open class DependencyGuardListTask : DefaultTask() { | |||
| ) | |||
| } | |||
|
|
|||
| @get:Input | |||
| public abstract val shouldBaseline: Property<Boolean> | |||
There was a problem hiding this comment.
These are called "managed properties" and Gradle instantiates them for us.
| @Suppress("NestedBlockDepth") | ||
| @TaskAction | ||
| internal fun execute() { | ||
| requirePluginConfig( |
There was a problem hiding this comment.
This check now happens at the start of task execution instead of in configuration. This is the main real behavior change in the PR. The rest is basically refactoring.
autonomousapps
force-pushed
the
trobalik.domainobjects
branch
from
3d4a19f to
118ce9d
Compare
autonomousapps
changed the title
|
It's failing the API check again (which is expected). |
autonomousapps
force-pushed
the
trobalik.domainobjects
branch
from
118ce9d to
a4ade1f
Compare
This draft PR is based on the test PR I made earlier (so you can ignore the first commit). I'm sharing this in its draft state to get your feedback on the general idea. In the end, the ultimate idea would be to remove the mutable list entirely from the extension.
One thing worth noting is that there's a subtle bug in the current implementation of the plugin. You use this function in your task configuration:
That will fail if
extension.configurationsis empty. You are implicitly assuming that thedependencyGuardDSL block will be evaluated before tasks get configured. I can imagine a scenario that triggers task configuration early, causing a failure even though the build script is correctly configured. It would be a complex scenario, but people do crazy things in their builds and plugins. I think you'd be better off delaying that check to task execution.