Skip to content

Use Gradle's task configuration avoidance APIs in the main reference docs #30000

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

Conversation

larsgrefer
Copy link
Contributor

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Feb 26, 2022
Copy link
Member

@wilkinsona wilkinsona left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @larsgrefer. I've left a couple of comments for your consideration when you have a minute.

@@ -33,7 +33,9 @@ If you are using an `additional-spring-configuration-metadata.json` file, the `c

[source,gradle,indent=0,subs="verbatim"]
----
compileJava.inputs.files(processResources)
tasks.named('compileJava') {
inputs.files(processResources)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think will eagerly create the processResources task.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will only create the processResources task when the closure is executed, which in turn only happens when compileJava is being created/configured. In this case, processResources would have to be created anyway.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if that is that the case 100% of the time.

Regardless, this is an example of the increased complexity of task configuration avoidance — you need to be more aware of the relationships between tasks. According to Gradle’s own docs, you also need to consider where the task was created.

In this case processResources is defined by another plugin, but so is compileJava. That makes Gradle’s guidance a little unclear to me.

My feeling is that consistently using tasks.named in a situation such as this reduces cognitive load. You don’t need to think about where a task has come from or their existing relationships and things will just work (I think).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my tests it did not make a difference if processResources is used lazily or directly (inside the lazily evaluated compileJava closure). The nearest match in the Gradle docs I could find is this:

tasks.register("someTask")

tasks.register("anEagerTask") {
    dependsOn someTask
}

Here someTask is referenced directly inside the lazy configuration closure of anEagerTask.

That being said, I agree that just always using tasks.named makes things more consistent.

@@ -71,7 +71,7 @@ You can automatically expand properties from the Gradle project by configuring t

[source,gradle,indent=0,subs="verbatim"]
----
processResources {
tasks.named('processResources', Copy) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the task's type is needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it would work without the type.

I've added the explicit type declaration to give the IDE a chance to resolve what expand refers to, but at least for IntelliJ IDEA this does not seem to be sufficient.

For proper auto-completion in IntelliJ IDEA we'd need this:

tasks.named('processResources', Copy) {
    it.expand(project.properties)
}

If we don't care about IDE support, this would be enough:

tasks.named('processResources') {
    expand(project.properties)
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Given that it’s unrelated to configuration avoidance, please revert this part of the changes.

@wilkinsona wilkinsona added the status: waiting-for-feedback We need additional information before we can continue label Feb 27, 2022
@larsgrefer
Copy link
Contributor Author

I've updated the PR based on your comments.

@spring-projects-issues spring-projects-issues added status: feedback-provided Feedback has been provided and removed status: waiting-for-feedback We need additional information before we can continue labels Mar 2, 2022
@wilkinsona wilkinsona changed the title Use Gradles task configuration avoidance APIs in the reference docs Use Gradle's task configuration avoidance APIs in the reference docs Mar 3, 2022
@wilkinsona wilkinsona added this to the 2.5.x milestone Mar 3, 2022
@wilkinsona wilkinsona added type: documentation A documentation update and removed status: waiting-for-triage An issue we've not yet triaged status: feedback-provided Feedback has been provided labels Mar 3, 2022
@wilkinsona wilkinsona self-assigned this Mar 3, 2022
@wilkinsona wilkinsona changed the title Use Gradle's task configuration avoidance APIs in the reference docs Use Gradle's task configuration avoidance APIs in the main reference docs Mar 3, 2022
@wilkinsona wilkinsona closed this in 2b98fce Mar 3, 2022
@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.5.11 Mar 3, 2022
@wilkinsona
Copy link
Member

Thanks very much, @larsgrefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation A documentation update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants