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.

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

Avoid resolving dependencies at configuration time #264

Merged
merged 1 commit into from
Oct 8, 2020

Conversation

jdai8
Copy link
Contributor

@jdai8 jdai8 commented Oct 7, 2020

Fixes #263.

In most cases, checking the dependency tree is not necessary, as shouldRunAfter does not add any tasks to the task graph. I think it's simpler to add the task ordering to all classes tasks in the project.

However, this does change the behavior in certain cases. For example, if someone runs ./gradlew classes composeUp with no isRequiredBy, currently the plugin will have no ordering between the two tasks. These use cases seem unlikely to me though.

I can change the implementation to use taskGraph.whenReady if you think that's better. That implementation is just harder to unit test, as it relies on Gradle's API.

@augi augi self-assigned this Oct 8, 2020
@augi augi self-requested a review October 8, 2020 07:37
@augi
Copy link
Member

augi commented Oct 8, 2020

Hello, thank you for this PR!

I wasn't sure what Gradle does if dependsOn and shouldRunAfter are in conflict. But it looks like it is OK, probably because shouldRunAfter is just a kind of recommendation.

task taskA { doLast { logger.lifecycle('A') } }
task taskB { doLast { logger.lifecycle('B') } }
tasks.taskA.dependsOn tasks.taskB
tasks.taskB.shouldRunAfter tasks.taskA

@augi augi merged commit bf7f73d into avast:master Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

isRequiredBy resolves dependencies at configuration time
2 participants