Specify Automatic-Module-Name for kotlinx-coroutines-core #4320
Conversation
Definitely would vote for "common" or "metadata" over "trampoline". |
There was a problem hiding this comment.
I can confirm that the reproducer provided at #3842 no longer has the described issue after these changes.
My understanding of Java modules is surface-level, so please double-check my understanding. Right now, both kotlinx-coroutines-core and kotlinx-coroutines-core-jvm have the Java module name name kotlinx.coroutines.core; core-jvm specifies it explicitly, and -core just gets it automatically. If someone depends on core, they also depend on core-jvm, so any mentions of kotlinx.coroutines.core in Java module specifications will now correctly resolve to core-jvm (and not to both), but in addition to that, kotlinx.coroutines.core.trampoline will refer to a module containing nothing of use, so no one has any reason to write kotlinx.coroutines.core.trampoline in their code, ever. Right?
If so, the name I'd prefer would be scary and highlight the uselessness of the module. Something like kotlinx.coroutines.core.artifact_disambiguating_module.
| @@ -85,7 +85,7 @@ Add dependencies (you can also add other modules that you need): | |||
| ```xml | |||
| <dependency> | |||
| <groupId>org.jetbrains.kotlinx</groupId> | |||
| <artifactId>kotlinx-coroutines-core</artifactId> | |||
| <artifactId>kotlinx-coroutines-core-jvm</artifactId> | |||
There was a problem hiding this comment.
In my opinion, -jvm is visual noise in this case and doesn't actually mean anything. Of course it's JVM, we're mentioning a Kotlin project in a Maven file. I understand that there's an extra dependency resolution step, but does it have any non-negligible effect?
There was a problem hiding this comment.
I understand that there's an extra dependency resolution step, but does it have any non-negligible effect?
Just trying to make the world better by saving ~130KB of networking traffic for each resolution when the artifact is not cached locally yet. :)
There was a problem hiding this comment.
However, you're also adding 4 bytes to every load of the README file or the title page of kotlinx.coroutines; also, 4 bytes are going to be added to every version of README.md in the git repository starting from this PR (to be fair, those blob objects are compressed, so this is less critical).
Yes, you're absolutely correct.
That'd work for me. |
| @@ -85,7 +85,7 @@ Add dependencies (you can also add other modules that you need): | |||
| ```xml | |||
| <dependency> | |||
| <groupId>org.jetbrains.kotlinx</groupId> | |||
| <artifactId>kotlinx-coroutines-core</artifactId> | |||
| <artifactId>kotlinx-coroutines-core-jvm</artifactId> | |||
There was a problem hiding this comment.
However, you're also adding 4 bytes to every load of the README file or the title page of kotlinx.coroutines; also, 4 bytes are going to be added to every version of README.md in the git repository starting from this PR (to be fair, those blob objects are compressed, so this is less critical).
Open questions are:
kotlinx.coroutines.core.metadatawill be better.kotlinx-coroutines-coredependency in the readme file? I don't see any immediate benefits, but I see a downside:-coredependency adds an extra dep resolution step.Fixes #3842