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

Apply version catalog in the convention plugins #1517

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

Jaehwa-Noh
Copy link
Contributor

Apply version catalog in the convention plugins.

- hilt-plugin
- jacoco
- kotlin-android

Change-Id: Id6c32a6ebbd0b61b589b860facacddd10c9146ec
Change-Id: I471d999ca3bd9c9246b24565d6e2f57bde010225
@dturner
Copy link
Collaborator

dturner commented Jul 2, 2024

Please explain why this is a good idea. At first glance, the lines like this:

libs.findPlugin("compose").get().get().pluginId

look less readable than the original

apply(plugin = "org.jetbrains.kotlin.plugin.compose")

@Jaehwa-Noh
Copy link
Contributor Author

@dturner I think this is the good for SSOT, and when we would change plugins, we can change name or group in just one place libs.versions.toml.
I agreed with you it is hard to read then origin, but I think that we can remove .get().get() using extension function.

Kindly leave your comment here. I'll change or close this PR.
Thank you.

Change-Id: I40fb8cc7e13cf8fcf38d9f7fcb8910e0a5f144c2

# Conflicts:
#	build-logic/convention/src/main/kotlin/AndroidFeatureConventionPlugin.kt
#	build-logic/convention/src/main/kotlin/HiltConventionPlugin.kt
- VersionCatalog.getPlugin

Change-Id: I9731877a9a583d3428b600bfdbcffc016facd768
Change-Id: Iaf06e86fdb8f98d1537bbbdbb05d22c38f6551b4
@Jaehwa-Noh
Copy link
Contributor Author

@dturner
I have improved the style more readable.
It have had more clear context about the source, it comes from version catalog.

apply(plugin = libs.getPlugin("android.application"))

Is there anything more do I improve?

Change-Id: If49856693c5ce13a77421842b03642badc7c53c3
Change-Id: Ibbb6190b256bcbab8f1602745a8c6615639ca91d
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.

2 participants