Add KOTLIN_SCRIPTS feature flag #583
Conversation
There was a problem hiding this comment.
A few inline comments.
Also, a general thought I had for a while: We probably want to not depend on being a Gradle plugin for Kotlin Scripts support.
That means potentially a lot of refactoring to make a Gradle independent core.
We can sure start with a version that depends on the Gradle plugin and a host Gradle project to run, but that's not something we want in the future I think.
If you want to iterate on that Kotlin scripts support soon, I suggest that we do a little brainstorming session on the phone, or IRL when I'll be in Paris on the 3rd of November, and I write an issue or a design doc about the considerations for a first version of the feature. That should help us avoid a long to review and fix PR like what happened for versions catalog support.
plugins/core/src/main/kotlin/de/fayard/refreshVersions/core/RefreshVersionsCleanupTask.kt
Outdated
Show resolved
Hide resolved
plugins/core/src/main/kotlin/de/fayard/refreshVersions/core/RefreshVersionsTask.kt
Outdated
Show resolved
Hide resolved
| @Input | ||
| @Optional | ||
| @Option(option = "enable", description = "Enable a feature flag") | ||
| var enableFlag: FeatureFlag? = null | ||
| set(value) { | ||
| field = value | ||
| if (value != null) FeatureFlag.userSettings.put(value, true) | ||
| } | ||
|
|
||
| @Input | ||
| @Optional | ||
| @Option(option = "disable", description = "Disable a feature flag") | ||
| var disableFlag: FeatureFlag? = null | ||
| set(value) { | ||
| field = value | ||
| if (value != null) FeatureFlag.userSettings.put(value, false) | ||
| } |
There was a problem hiding this comment.
I think this should be in a separate PR, don't you?
There was a problem hiding this comment.
actually no, because it's a copy/paste from RefreshVersionsTask that is known to work, and because adding the flag is my main focus here, see comment below
|
@LouisCAD I commented on your idea to have something independant from Gradle here #586 I still think there is no issue with adding Kotlin Script support in the current codebase. Something I need to clarify is that I do not want to implement Kotlin Scripts support in one large pull request here like I did for Gradle Versions Catalogs. I learned my lesson that small is beautiful, and having multiple small pulll requests is less work and less stress for everyone that one huge pull request that does everything. |
Co-authored-by: Louis CAD <[email protected]>
Co-authored-by: Louis CAD <[email protected]>
|
I don't think that getting this PR merged or not is an actual blocker to start working/iterating on the kotlin scripts support feature, but since we might want to add further feature flags at some point, which would generate little git conflicts, I guess it's a good idea to merge it now. |
Minimum viable PR for #582
Discussion should happen in the issue