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

Improve DSL by leveraging Gradle managed objects #600

Closed
gmazzo opened this issue Apr 18, 2024 · 2 comments · Fixed by #625
Closed

Improve DSL by leveraging Gradle managed objects #600

gmazzo opened this issue Apr 18, 2024 · 2 comments · Fixed by #625
Assignees
Labels
Feature Feature request issue type S: ready for release Status: merged in the main branch

Comments

@gmazzo
Copy link

gmazzo commented Apr 18, 2024

What is your use-case and why do you need this feature?

I recently put focus into this project while I was researching it for a potential replacement on JaCoCo.

While exploring 0.8.0-Beta2 usage and internals (source code), I've identified some pain points in the current experimental DSL, that IMHO should be addressed by leveraging Gradle-managed objects, like NamedDomainObjectContainer and its variants.

I didn't scan fully what the project has to offer, like filtering, etc. So you may consider this as partial feedback.

Describe the solution you'd like

Child closures in DSL

The main kover: KoverProjectExtension extension has several child closures, like reports: KoverReportsConfig.

These closures can only be accessed by calling a method (i.e. kover.reports { }).

As my current understanding of Gradle's patterns, child closures should be accessible by property or method closure, so the following two syntaxes should be valid and equivalent:

kover.reports.doSomething()
// and
kover.reports {
   doSomething()
}

This can easily be achieved by changing a bit the DSL object definition, i.e.:

interface KoverProjectExtension {

    val reports: KoverReportsConfig
    
    fun reports(action: Action<KoverReportsConfig>) = action.configure(reports)

}

kover.variants DSL

Kover currently supports (as far I noted) three types of variants: provided, customer, and total

It would be nice if you iterate the DSL to be able to access all declared variants with a NamedDomainObjectSet (at least), or even better, use a PolymorphicDomainObjectContainer to also use Gradle's standard API for creating them.

For instance, having:

interface KoverProjectExtension {
    val variants: PolymorphicDomainObjectContainer<KoverVariant>
}

interface KoverVariantsContainer : PolymorphicDomainObjectContainer<KoverVariant> {
    val provided: NamedDomainObjectCollection<KoverProvidedVariant>
    val custom: NamedDomainObjectCollection<KoverCustomVariant>
    val total: KoverTotalVariant
}

// minimal DSL interfaces
interface KoverVariant: Named
interface KoverProvidedVariant : KoverVariant
interface KoverCustomVariant : KoverVariant
interface KoverTotalVariant : KoverVariant

This could be a basic implementation for KoverVariantsContainer:

internal abstract class KoverVariantsContainerImpl @Inject constructor(
    objects: ObjectFactory,
) : KoverVariantsContainer,
    PolymorphicDomainObjectContainer<KoverVariant> by (objects.polymorphicDomainObjectContainer(KoverVariant::class).apply {
        // only `KoverCustomVariant`s can be registered through DSL
        registerBinding(KoverCustomVariant::class.java, KoverCustomVariant::class.java)
    }) {

    override val provided = withType<KoverProvidedVariant>()

    override val custom = withType<KoverCustomVariant>()

    override val total = objects
        .newInstance<KoverTotalVariant>("total")
        .also(::add)

    internal fun addProvidedVariant(name: String, action: Action<KoverProvidedVariant>) = objects
        .newInstance<KoverProvidedVariant>(name)
        .also(action::execute)
        .also(::add)
}

You could have a richer DSL, supporting many use cases out of the box:

Iterating all variants in a hot collection

kover.variants.configureEach {
    sources.excludedSourceSets += "main"
}

Configuring variant in a generic way

Imaging #599 is implemented in a way that KoverVariant now has an aggregate property.

We can target all debug variants at once with:

kover.variants.provided.configureEach {
    aggregate = name.lowercase().endsWith("debug")
}

Simplified plugin internal wiring

Currently, your plugin has some manual checks to prevent variants of different types from collishing with others with the same name.

If you centralize that into a root NamedDomainObjectContainer, this problem will be gone (less code).

Later the plugin can just react on others, like Android one, and automatically register provided ones:

val extension = project.extensions.create<KoverProjectExtension>("kover")

androidComponents.onVariants variant@{
    val koverVariant = extension.variants.addProvidedVariant(this@variant.name) {
       // configure koverVariant
    }
}
@gmazzo gmazzo added Feature Feature request issue type S: untriaged Status: issue reported but unprocessed labels Apr 18, 2024
@shanshin
Copy link
Collaborator

Thanks for the detailed feedback!

@shanshin shanshin added S: confirmed Status: bug is reproduced or present and removed S: untriaged Status: issue reported but unprocessed labels Apr 23, 2024
shanshin added a commit that referenced this issue Jun 7, 2024
The new DSL will allow to write chains like `kover.reports.total { ... }` instead of `kover { reports { total { ... } } }`.

However, the DomainObjectContainer for variants has not been implemented, because it will be necessary to rework the workflow with different types of variants, which is part of a larger reworking of the DSL.

Resolves #600
shanshin added a commit that referenced this issue Jun 7, 2024
The new DSL will allow to write chains like `kover.reports.total { ... }` instead of `kover { reports { total { ... } } }`.

However, the DomainObjectContainer for variants has not been implemented, because it will be necessary to rework the workflow with different types of variants, which is part of a larger reworking of the DSL.

Resolves #600
shanshin added a commit that referenced this issue Jun 7, 2024
The new DSL will allow to write chains like `kover.reports.total { ... }` instead of `kover { reports { total { ... } } }`.

However, the DomainObjectContainer for variants has not been implemented, because it will be necessary to rework the workflow with different types of variants, which is part of a larger reworking of the DSL.

Resolves #600
shanshin added a commit that referenced this issue Jun 7, 2024
The new DSL will allow to write chains like `kover.reports.total { ... }` instead of `kover { reports { total { ... } } }`.

However, the DomainObjectContainer for variants has not been implemented, because it will be necessary to rework the workflow with different types of variants, which is part of a larger reworking of the DSL.

Resolves #600
shanshin added a commit that referenced this issue Jun 7, 2024
The new DSL will allow to write chains like `kover.reports.total { ... }` instead of `kover { reports { total { ... } } }`.

However, the DomainObjectContainer for variants has not been implemented, because it will be necessary to rework the workflow with different types of variants, which is part of a larger reworking of the DSL.

PR #625
Resolves #600
@shanshin shanshin reopened this Jun 7, 2024
@shanshin shanshin added S: ready for release Status: merged in the main branch and removed S: confirmed Status: bug is reproduced or present labels Jun 7, 2024
@shanshin
Copy link
Collaborator

shanshin commented Jun 10, 2024

Partially resolved in 0.8.1, the rest of the suggestions are written in #608 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature Feature request issue type S: ready for release Status: merged in the main branch
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants