Deprecate declaring multiple targets of the same type #3690
Conversation
|
This is currently in a draft state. We might adjust it with more real use cases. |
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days. |
antohaby
force-pushed
the
alakotka/declaring-multiple-targets
branch
from
a21f8a9
to
6136891
Compare
multiple targets
antohaby
force-pushed
the
alakotka/declaring-multiple-targets
branch
from
6136891
to
5aa8101
Compare
There was a problem hiding this comment.
Thanks for providing examples, and sorry for suggesting to drop this work, but I'm really concerned that they are not "complete", but still quite verbose (and if we want to make them "complete", they'll have to be even more verbose). Let's discuss the idea of dropping them completley?
| kotlin { | ||
| jvm() | ||
| jvm("jvmKtor") | ||
| jvm("jvmOkio") // is not recommended and will produce deprecation warning |
There was a problem hiding this comment.
It reads as if the previous line (jvm("jvmKtor")) is fine, but it's not, right?
There was a problem hiding this comment.
right, we can remake it to just:
jvm()
jvm("jvmKtor") // is not recommended...
| ```kotlin | ||
| // Project ':myLibrary:integrationTests' | ||
| kotlin { | ||
| jvm("jvmKtor") // !!! |
There was a problem hiding this comment.
I'd recommend to avoid using JVM-shared source sets without the specific the need. I.e., let's add linuxX64 or something here.
| val okioIntegrationTest by compilations.creating | ||
|
|
||
| // Associate with test compilation to see tests from commonTest | ||
| ktorIntegrationTest.associateWith(compilations.getByName("test")) |
There was a problem hiding this comment.
Is associateWith considered non-experimental for KMP? I thought it's not, and only associateWith in JVM is.
| val okioIntegrationTest by compilations.creating | ||
|
|
||
| // Associate with test compilation to see tests from commonTest | ||
| ktorIntegrationTest.associateWith(compilations.getByName("test")) |
There was a problem hiding this comment.
Why is it associated with test? Does it even work in IDE?
I think the direct counterpart for the previous snippet would be to:
- associate new compilations with
main - connect link their default source sets with
dependsOnto thecommonTest
| // Project ':myUILibrary' | ||
| kotlin { | ||
| jvm("jvmCompose") // !!! | ||
| jvm("jvmSwing") // !!! |
There was a problem hiding this comment.
Ditto about having commonMain as JVM-shared source set
| ```kotlin | ||
| // Project ':myUILibrary | ||
| kotlin { | ||
| jvm() |
There was a problem hiding this comment.
I'd also avoid promoting single-target projects
| </tab> | ||
| <tab title="Now" group-key="now"> | ||
|
|
||
| ```kotlin |
There was a problem hiding this comment.
I'm a bit conernced that this example is neither concise, nor complete. I think we have to make a firm decision: either we're providing complete examples, or we don't provide them at all and just assume that the users that had multiple same targets are advanced enough to handle this refactoring on their own.
I'm leaning towards the latter. Mostly because it's so hard to give a proper minimal migration example -- as it involves multiple files in the project, the only solution I see is an actual GitHub project with "migration commit", but IMO it's too much for this case (but we can expand it, if we see demand/questions)
There was a problem hiding this comment.
Agree with @dsavvinov here. Also, it would be nice to include a paragraph about when to choose separate compilations over separate Gradle projects. Adding a separate Gradle project is usually more involved. Unless there's something fundamental it unlocks, I would even not mention it.
| // Create a Test Run to for each integration test | ||
| testRuns { | ||
| create("ktorIntegration") { setExecutionSourceFrom(ktorIntegrationTest) } | ||
| create("okioIntegration") { setExecutionSourceFrom(okioIntegrationTest) } |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Changes in this request will be merged as part of the #3789 . I'm closing this one. |
Hi!
I want to add deprecation information about declaring multiple targets in Kotlin Multiplatform Projects.
YT issue about deprecation: https://youtrack.jetbrains.com/issue/KT-59316
I also would like to ask your advice on how to structure this information best and consistently.