Skip to content
This repository has been archived by the owner on Feb 11, 2022. It is now read-only.

Introduce lazy tasks except for ktlint #184

Merged
merged 21 commits into from May 13, 2019

Conversation

tasomaniac
Copy link
Contributor

@tasomaniac tasomaniac commented May 4, 2019

Finally finished with lazy tasks except for ktlint.

Ktlint is more challenging since we support multiple versions. That's why we need to resolve tasks to identify.

For Checkstyle, PMD and Findbugs, I've changed CodeQualityConfigurator a lot. How we configure tasks is completely redesigned.

  • Now we create collectViolations tasks per sourceSet
  • Then collectViolationsVariant as a meta task to trigger others. This one is also used for variantFilter.

Android Lint was easier. Just used register instead of maybeCreate. But I had to hold on a flag to make sure that the task is created only once. There is no equivalent maybeRegister in Gradle.

Detekt is also similar. But now, I've realized that we don't support configuring multiple times. I will do a follow-up to support that.

without anything with Checkstyle, Pmd, Findbugs, Lint
image image

Notice that the number of tasks configured is the same (only 23)

@tasomaniac tasomaniac requested a review from mr-archano May 4, 2019 12:58
@tasomaniac
Copy link
Contributor Author

Guess running too many PRs ws too much for the machine. Gradle daemon disappeared. Can someone retry the task please?

@tasomaniac tasomaniac changed the base branch from master to develop May 10, 2019 21:31
protected abstract void createToolTaskForAndroid(sourceSet)

protected void configureToolTask(T task) {
sourceFilter.applyTo(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.

Collected all common functionality here. It is automatically applied to all tool tasks thanks to configureEach in line 53 above.

Copy link
Contributor

@mr-archano mr-archano left a comment

Choose a reason for hiding this comment

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

LGTM already, a couple of questions.

}
}

protected final String getToolTaskNameFor(sourceSet) {
Copy link
Contributor

Choose a reason for hiding this comment

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

technically this isn't always a sourceset right? maybe we can just called it named?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Soooo, this was actually only for sourceSet. Then I realized that FindBugs needs to be configured per variant because it needs javaCompile task. I will go over and make sure that naming is consistent.


class TasksCompat {

private static boolean IS_GRADLE_MIN_49 = GradleVersion.current() >= GradleVersion.version("4.9")
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I'd prefer ' over " 💃

protected void configureToolTask(T task) {
sourceFilter.applyTo(task)
task.group = 'verification'
task.exclude '**/*.kt'
Copy link
Contributor

Choose a reason for hiding this comment

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

is this ok because this configurator is bound to SourceTask & CodeQualityExtension?

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, here exclude method comes from SourceTask.

@mr-archano mr-archano merged commit 4b30fb4 into develop May 13, 2019
@mr-archano mr-archano deleted the taso/lazy-tasks-except-for-ktlint branch May 13, 2019 09:38
@tasomaniac tasomaniac mentioned this pull request May 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants