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

✚ plural type-safe way of adding dependencies #229

Closed
wants to merge 1 commit into from

Conversation

jmfayard
Copy link
Member

@jmfayard jmfayard commented Aug 26, 2020

Augment the Kotlin DSL for the block dependencies { }
with its plural forms implementations, apis and testImplementations.

For example:

dependencies {
  implementations(KotlinX.coroutines.core, KotlinX.coroutines.android)
}

This is both more concise and more type-safe.
implementation(KotlinX.coroutines) is wrong but would fail only at runtime because the method accept an object as parameter

Voice____IdeaProjects_android_Voice__-_build_gradle_kts___data_

Augment the Kotlin DSL for the block `dependencies { }`
with its plural forms "implementations", "apis", "testImplementations".

For example:

```
dependencies {
  implementations(KotlinX.coroutines.core, KotlinX.coroutines.android)
  // or
  implementations(listOf(KotlinX.coroutines.core, KotlinX.coroutines.android))
}
```

This is both more concise and more type-safe.
`implementation(KotlinX.coroutines)` is wrong but would fail only at runtime because the method accept an object as parameter
@jmfayard jmfayard requested a review from LouisCAD August 26, 2020 12:16
Copy link
Member

@LouisCAD LouisCAD left a comment

Choose a reason for hiding this comment

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

Having multiple dependencies declarations makes eye scanning harder, as you need to juggle back and forth in addition to scanning down, and you lose the alignment.

The type safety is a nice thing though I like it. I think this is an area that deserves exploration as your proposal is not the only possible solution.

Maybe we should open an issue about that, or discuss this in another channel?

BTW, for future PRs, let's target the develop branch so people always browse the code reflecting the latest stable release by default.

@jmfayard jmfayard closed this Aug 29, 2020
jmfayard added a commit that referenced this pull request Aug 29, 2020
This is a follow up on #229

It focuses on typesafety only

There is an elegant Kotlin solution for every problem.

Since implentation is defined like this

```
fun DependencyHandler.`implementation`(dependencyNotation: Any): Dependency? =
    add("implementation", dependencyNotation)
```

Then we can overwrite this function for when the parameter is IsNotADependency or DependencyNotationAndGroup

This way the IDE shows you that made a mistake
jmfayard added a commit that referenced this pull request Aug 31, 2020
This is a follow up on #229

It focuses on typesafety only

There is an elegant Kotlin solution for every problem.

The IDE now imports two version of `implementation`. 

```
fun DependencyHandler.`implementation`(dependencyNotation: Any): Dependency? =
    add("implementation", dependencyNotation)

@kotlin.Deprecated(ErrorIsNotADependency)
fun DependencyHandler.`implementation`(dependencyNotation: IsNotADependency): Dependency? =
    null
```

Since the second one is more specific, it takes precedence when the parameter is `IsNotADependency` or in practice `DependencyNotationAndGroup`

This way the IDE shows you that made a mistake

<img width="874" alt="GraySky____IdeaProjects_gray-sky-weather__-_build_gradle_kts___app_" src="https://user-images.githubusercontent.com/459464/91646121-1b9e6180-ea4c-11ea-87b1-c18cd08c63b1.png">
@jmfayard jmfayard deleted the add-dependencies branch October 15, 2020 13:28
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.

None yet

2 participants