-
Notifications
You must be signed in to change notification settings - Fork 5
Conversation
…n dsl files to identify the project type
ktlint fixes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@programadorthi Thanks for the PR! Looks REALLY good, you've managed to tackle one improvement I was just too lazy to deliver 😹
I've requested two small changes, most cosmetic. I've added one question as well.
...odules-plugin/src/test/kotlin/io/labs/dotanuki/magicmodules/tests/MagicModulesPluginTests.kt
Outdated
Show resolved
Hide resolved
gradle.settingsEvaluated { | ||
val parsedStructure = ProjectStructureParser(extension).parse(settingsDir) | ||
val processedScripts = BuildScriptsProcessor.process(parsedStructure) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you share the practical reasons why the instances now are created per settings evaluation?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main reason is that plugin configurations are lazy. When we apply a plugin, the apply function is called at the same time and only default values are available in the configuration lifecycle. So, we have to use a lazy function as doLast
, settingsEvaluated
, in this case, to get custom values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense to me!
private const val NO_NAME_ASSIGNED = "" | ||
private val APPLY_FROM_LINE_REGEX = """^\s*apply\s*\(?from\s*[:=]\s*['"]?""".toRegex() | ||
private val PLUGIN_LINE_REGEX = | ||
"""^\s*((apply\s*\(?\s*plugin)|(id\s*[('"])|(kotlin\s*\())""".toRegex() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏👏👏👏👏
import io.labs.dotanuki.magicmodules.internal.model.GradleModuleType | ||
import io.labs.dotanuki.magicmodules.internal.model.GradleProjectStructure | ||
import io.labs.dotanuki.magicmodules.internal.util.e | ||
import io.labs.dotanuki.magicmodules.internal.util.i | ||
import io.labs.dotanuki.magicmodules.internal.util.logger | ||
import java.io.File | ||
|
||
internal class ProjectStructureParser { | ||
internal class ProjectStructureParser(private val magicModulesExtension: MagicModulesExtension) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will prefer not depend on the extension here. Could you please change this constructor and pass the list of raw plugins directly?
I know we'll have to wire things at plugin entry point level, but I think this abstraction becomes better!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alright. I'll change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I created a data class to parse the raw contents to Parser. Reasons:
- Avoid
ProjectStructureParser
constructor with 3 parameters of the same type; - Avoid constructor to have long parameter list;
- Improve maintenance.
created a data class to avoid mistakes and help with maintenance
created a data class to avoid mistakes and help with maintenance
Added support to customize matches to grab more gradle plugin declarations. Some people use raw text when applying plugins. Other are using Kotlin DSL and constant fields to reuse plugin names in whole project. So, it was added fields to the plugin extension that they can be used to match plugin declarations.
For instance:
Kotlin DSL:
Groovy DSL:
As the plugin only matches the raw text, looking for
"com.android.application"
by instance will never match. So, we need ask the plugin user how to matches plugins declarations in this way:rawApplicationPlugin
- how to matches application plugins? default value is["com.android.application"]
.rawLibraryPlugins
- how to matches library plugins? default values are["com.android.library", "kotlin", "com.android.dynamic-feature", "jvm"]
There is a extra field called
rawLibraryUsingApplyFrom
used in scenarios when users that create a base script and apply to other modules. E.g:So, just config the property setting how to grab them: