Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Avoid eager creation of tasks for Spotless plugin #617

Merged
merged 3 commits into from
Jun 18, 2020
Merged

Avoid eager creation of tasks for Spotless plugin #617

merged 3 commits into from
Jun 18, 2020

Conversation

bigdaz
Copy link
Contributor

@bigdaz bigdaz commented Jun 18, 2020

Previously applying the Spotless plugin resulted in a many tasks to be created
per subproject, even for an invocation like gradle help.
Using "Task configuration avoidance" APIs means that these tasks will
not be constructed if they will not be required during task execution.

bigdaz added 3 commits June 18, 2020 15:21
@bigdaz
Do not eagerly create format-specific spotless tasks
ef2aae4 
Previously applying the Spotless plugin resulted in a many tasks to be created
per subproject, even for an invocation like `gradle help`.
Using "Task configuration avoidance" APIs means that these tasks will
not be constructed if they will not be required in the task graph.
@bigdaz
Do not eagerly create per-project spotless tasks
621af48 
With this commit, running `./gradlew help -PspotlessModern=true` will no longer
the `spotless`, `spotlessCheck` or `spotlessApply` tasks if they are not
required for task execution. The only remaining task that is eagerly constructed
is "spotlessInternalRegisterDependencies".
@bigdaz
Update TODO items in code
78cfd65
@nedtwigg
Copy link
Member

This LGTM, merge at will! If you have code-in-progress for the Provider<> stuff, I'm eager to see it. Or if you'd like me to send up a PR for us to look at with the // TODO override configure(String name, Class<T> clazz, Action<T> configure) so that it is lazy idea, I'm happy to do that.

@bigdaz
Copy link
Contributor Author

bigdaz commented Jun 18, 2020

Thanks Ned. In the end I didn't bother trying to get rid of the afterEvaluate and the migration to Task config avoidance APIs was pretty straightforward.

I'm not sure what you intend for making configure lazy, so I might leave that to you.

FYI I'm having an issue running spotlessApply on the master branch in my fork. I suspect it's got to do with the fact that it was renamed to main in the primary repo, but I'm not sure. Here's a build scan with the failure: https://scans.gradle.com/s/svlbtvyqgiwj2/failure. Once the PR is merged I'll see what I need to do to get it working, but I presume a NPE isn't the intended behaviour :).

@bigdaz bigdaz merged commit 1cc5f54 into diffplug:main Jun 18, 2020
@bigdaz bigdaz deleted the lazy-task-creation branch June 18, 2020 23:02
@bigdaz
Copy link
Contributor Author

bigdaz commented Jun 18, 2020

I can confirm that by deleting and re-creating my fork on GitHub, the above issue is resolved.

@nedtwigg
Copy link
Member

Yup, def some transients in the master -> main transition :). I'll take a quick crack at the lazy idea I had, and I'll throw it up to get your feedback on its pros/cons vs Provider<> approaches.

@nedtwigg
Copy link
Member

Part of #601

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@bigdaz @nedtwigg