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

Use variants dynamically to put sourceSets #167

Closed

Conversation

tasomaniac
Copy link
Contributor

Instead of resolving sourceSets from extension, resolve variants and use the sources defined in there.

Fixes #153

target.files(Callable { androidSourceSet.java.srcDirs })
)
}
ext.variants.all { variant ->
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 ext.variants is coming from the PluginUtils.

Here all makes it lazy loop over variants. When a variant is created, it is sourceSet is also set and contains all directories.

@JLLeitschuh
Copy link
Owner

Is there any good way to add a unit test for this?

@tasomaniac
Copy link
Contributor Author

Ktlint integration with a Kotlin project is tested already in the repo. The same should be done for an Android project too. We have some tests in Novoda plugin, I used those test to double check these changes.

https://github.com/novoda/gradle-static-analysis-plugin/blob/develop/plugin/src/test/groovy/com/novoda/staticanalysis/internal/ktlint/KtlintIntegrationTest.groovy

If a setup is done, I would be happy to contribute to tests.

@JLLeitschuh
Copy link
Owner

@Tapchicoma Can unit tests against android be written or would that couple our test infrastructure to requiring the Android SDKs to be installed?


internal val BaseExtension.variants
get(): DomainObjectSet<out BaseVariant> = when (this) {
is AppExtension -> applicationVariants
Copy link
Contributor Author

Choose a reason for hiding this comment

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

As @Tapchicoma mentioned, this extension is not available on InstantApp modules. Don't really know what to do in this case.

Copy link
Owner

Choose a reason for hiding this comment

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

Can you elaborate on what the problem is?
I'm not an Android developer. Hahah.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Android have multiple plugins. In KtlintPlugin.kt class where we configure the Android plugin, we support InstantApp modules. But here, unfortunately we cannot access the variants in InstantAppExtension because it is not visible for some reason.

I've done this a lot of times but I never supported instant app plugin before. Not sure what to do in this case.

@Tapchicoma
Copy link
Collaborator

@tasomaniac thank you for your contribution!

As we discussed in #153 and #170 - can you update your PR:
Instead of creating check task for all android variant sources, create a meta task per variant (adding Variant for a task name - for example, ktlintKellerbierDebugVariantCheck) and include a dependency on specific source set task. As far, as I see from your PR, it will be possible to do, just need to extract source set check and format tasks name creation based on SourceSet name and use it, when declaring dependency on check (format) task.

@tasomaniac
Copy link
Contributor Author

Hey, I believe that would be the best. But I need to start from scratch to do that. I will just close this so anybody can take it.

@tasomaniac tasomaniac closed this Dec 4, 2018
@Tapchicoma
Copy link
Collaborator

@tasomaniac I think, I will approach this after #180 will be merged 👍

@tasomaniac
Copy link
Contributor Author

Thanks @Tapchicoma

@tasomaniac tasomaniac deleted the taso/android-variants branch December 4, 2018 22:39
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.

Alternate sourceSets are not detected
3 participants