Add meta tasks for Android variants #183

Tapchicoma · 2018-12-09T20:04:29Z

This PR adds android variants meta tasks, that trigger tasks for all source sets in given variant.

For example running ktlintKellerbierDebugCheck meta task triggers following tasks:

$ ./gradlew :samples:android-app:ktlintKellerbierDebugCheck -m

:samples:android-app:ktlintDebugSourceSetCheck SKIPPED
:samples:android-app:ktlintKellerbierDebugSourceSetCheck SKIPPED
:samples:android-app:ktlintKellerbierSourceSetCheck SKIPPED
:samples:android-app:ktlintMainSourceSetCheck SKIPPED
:samples:android-app:ktlintKellerbierDebugCheck SKIPPED

Also added limited support for android projects in new Kotlin multiplatform plugin:

$ ./gradlew -m :samples:kotlin-mpp-android:ktlintKellerbierDebugAndroidLibCheck

:samples:kotlin-mpp-android:ktlintAndroidLibDebugSourceSetCheck SKIPPED
:samples:kotlin-mpp-android:ktlintAndroidLibKellerbierDebugSourceSetCheck SKIPPED
:samples:kotlin-mpp-android:ktlintAndroidLibKellerbierSourceSetCheck SKIPPED
:samples:kotlin-mpp-android:ktlintAndroidLibMainSourceSetCheck SKIPPED
:samples:kotlin-mpp-android:ktlintKellerbierDebugAndroidLibCheck SKIPPED

Currently variant task name is inconsistent with source sets tasks names, but that will be addressed in #182 issue.

Closes #170.

Now plugin adds variant specific tasks that depends on variant sourceSets check tasks. Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Tapchicoma · 2018-12-09T20:04:55Z

@tasomaniac may you also take a look on this PR?

tasomaniac

Looks good to me. Only have a small comment. Would be good to add tests for this. Should we craete an issue and link here?

tasomaniac · 2018-12-09T20:35:28Z

CHANGELOG.md

@@ -7,7 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).

 ## [Unreleased]
 ### Added
-  - ?
+  - Meta tasks to run check or format on all sources in android variant. (#170)


tasomaniac · 2018-12-09T20:39:04Z

plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/KtlintPlugin.kt

+        multiplatformTargetName: String? = null
+    ): Task {
+        val taskName = "ktlint${variantName.capitalize()}${multiplatformTargetName?.capitalize() ?: ""}Check"
+        return tasks.findByName(taskName) ?: task(taskName).apply {


Instead can you use tasks.maybeCreate() that conditionally creates a task and returns the already available task if so.

Unfortunately, all of these options are eagerly evaluated. I'd love if there was a way to lazily instantiate this task.

Don't worry about that though.

I'm not sure actually. findByName is also not lazy. maybeCreate looks like a drop-in replacement of what you do here.

At the end it is not crucial. Either is fine.

I think, this can addressed in next PR with task configuration avoidance. For now, I changed it to maybeCreate()

JLLeitschuh · 2018-12-10T15:16:44Z

plugin/src/main/kotlin/org/jlleitschuh/gradle/ktlint/KtlintPlugin.kt

    private fun setCheckTaskDependsOnKtlintCheckTask(project: Project, ktlintCheck: Task) {
        project.tasks.findByName("check")?.dependsOn(ktlintCheck)
    }

    /*
     * Helper functions used until Gradle Script Kotlin solidifies it's plugin API.
     */
-


I think this is a line of documentation for all lower lines, so this newline gap was intentional.

JLLeitschuh · 2018-12-10T15:18:31Z

One concern I want to bring up is the potential for accidental task name collisions.
Are there any cases that anyone can think of whereby declaring source sets in certain ways you could unintentionally get task name declaration collisions?

If there are, please figure out how to prevent these task name collisions.

tasomaniac · 2018-12-10T18:31:02Z

I don't see any problems. It will bring back the old functionality of the tasks back.

The breaking change would be that the type of the tasks has changed together with the report file destinations. We are using that actually but I will make the necessary changes to support the new structure. I guess the meta task wording in the changelog already captures this.

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Tapchicoma · 2018-12-12T20:38:55Z

One concern I want to bring up is the potential for accidental task name collisions.
Are there any cases that anyone can think of whereby declaring source sets in certain ways you could unintentionally get task name declaration collisions?

I agree with @tasomaniac that there is no problem with this.

JLLeitschuh · 2018-12-12T20:57:30Z

Okay, if you are both unconcerned, then go for it if this is ready to go.

tasomaniac · 2018-12-12T21:47:05Z

Nice. Are you guys going to release a new version now or does it need more stuff in?

Tapchicoma · 2018-12-13T13:43:09Z

@tasomaniac As next release will anyway break api - I also want to bump minimal supported Gradle version to 5.0.0 and add task configuration avoidance. Then we can do new 7.0.0 release.

tasomaniac · 2018-12-13T23:04:43Z

Hey about the task configuration avoidance: there is a way to make it available in a backward compatible way. I found this example from official docs: https://github.com/melix/jmh-gradle-plugin/blob/a034aa88805b7a06fa9c5a825d573554b2aa23e2/src/main/groovy/me/champeau/gradle/JMHPlugin.groovy#L289-L296

Especially with Kotlin extension functions, this can be done in a very nice way.

Tapchicoma added 3 commits December 9, 2018 14:22

Add android variant meta tasks for common android projects.

a71e72a

Now plugin adds variant specific tasks that depends on variant sourceSets check tasks. Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Add android variants meta tasks in multiplatform project.

29ee16c

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Update changelog and readme.

54c4a1c

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Tapchicoma requested a review from JLLeitschuh December 9, 2018 20:04

tasomaniac reviewed Dec 9, 2018

View reviewed changes

JLLeitschuh reviewed Dec 10, 2018

View reviewed changes

Tapchicoma added 2 commits December 12, 2018 21:34

Use maybeCreate.

9f3af5d

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Restore empty line after comment.

bb39745

Signed-off-by: Yahor Berdnikau <egorr.berd@gmail.com>

Tapchicoma merged commit dc5aa45 into JLLeitschuh:master Dec 12, 2018

Tapchicoma deleted the build-types-meta-tasks branch December 12, 2018 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add meta tasks for Android variants #183

Add meta tasks for Android variants #183

Tapchicoma commented Dec 9, 2018

Tapchicoma commented Dec 9, 2018

tasomaniac left a comment

tasomaniac Dec 9, 2018

tasomaniac Dec 9, 2018

JLLeitschuh Dec 10, 2018 •

edited

tasomaniac Dec 10, 2018

Tapchicoma Dec 12, 2018

JLLeitschuh Dec 10, 2018

JLLeitschuh commented Dec 10, 2018

tasomaniac commented Dec 10, 2018

Tapchicoma commented Dec 12, 2018

JLLeitschuh commented Dec 12, 2018

tasomaniac commented Dec 12, 2018

Tapchicoma commented Dec 13, 2018

tasomaniac commented Dec 13, 2018

Add meta tasks for Android variants #183

Add meta tasks for Android variants #183

Conversation

Tapchicoma commented Dec 9, 2018

Tapchicoma commented Dec 9, 2018

tasomaniac left a comment

Choose a reason for hiding this comment

tasomaniac Dec 9, 2018

Choose a reason for hiding this comment

tasomaniac Dec 9, 2018

Choose a reason for hiding this comment

JLLeitschuh Dec 10, 2018 • edited

Choose a reason for hiding this comment

tasomaniac Dec 10, 2018

Choose a reason for hiding this comment

Tapchicoma Dec 12, 2018

Choose a reason for hiding this comment

JLLeitschuh Dec 10, 2018

Choose a reason for hiding this comment

JLLeitschuh commented Dec 10, 2018

tasomaniac commented Dec 10, 2018

Tapchicoma commented Dec 12, 2018

JLLeitschuh commented Dec 12, 2018

tasomaniac commented Dec 12, 2018

Tapchicoma commented Dec 13, 2018

tasomaniac commented Dec 13, 2018

JLLeitschuh Dec 10, 2018 •

edited