Update Kotlin dependency to latest stable 1.7 #623
Conversation
1.6.21 -> 1.7.22. 1.8 is available as of Dec 28, 2022, but it was not immediately clear to me if this would be compatible.
|
Baseline Kotlin version dependencies need to be discussed somewhere; perhaps this issue is the place. The challenge is that we have to be quite conservative -- upgrading to latest patch of same minor version is fine, but jumping from 1.5 to 1.7 is a big change. This is different from the question of which versions module will be compatible with (we have CI matrix build to check that). So, @dinomite @k163377 would we want to try increasing Kotlin baseline for 2.15(.0)? Probably not to 1.7, but 1.6.x could be possibility. |
|
Perhaps we need to stay with |
|
@k163377 yes that is kind of my leaning too. Module itself supports Kotlin versions 1.4 - 1.7, and CI now checks this compatibility too. |
|
Apologies, I originally went to commit this against maser, which is on That being said, you can update the compiler/plugin version independent of the language version. So perhaps the better approach is to control the language version independently of the plugin version? |
This allows us to control API/language version independently of Kotlin plugin versions. See: https://kotlinlang.org/docs/maven.html#specifying-compiler-options
|
WDYT @k163377 @dinomite @viartemev ? |
|
I tried Personally, I don`t think it should be merged. I understand the desire to use a newer version of |
This is bit different than what he is saying: he is saying that the dependency would be to 1.7.x, but compiler behavior is specified to produce bytecode (?) that should work with Kotlin 1.5 too. However: this would still by default increase kotlin-core transitive dependency to 1.7 for everyone (unless they locally override it). I don't think we want to merge this for 2.15, although could start discussing increasing dependency to 1.6 for 2.16 (or 1.7 if community wants that instead). |
|
Oops, I guess I misunderstood. |
|
For clarity, you might want to read: https://kotlinlang.org/docs/maven.html#attributes-common-to-jvm-and-js.
To me it sounds like this would actually help prevent problems like #556. If this project is supposed to be Kotlin 1.4 compatible we can guarantee that by setting those properties to 1.4. |
|
Thank you @VirtualTim that makes sense and is what I thought. The main remaining question is whether we'd want to upgrade dependency since it will change observed dependencies downstream. I do not have strong opinion on this, other than to say that most users will consider it (incorrectly but understandably) as increasing minimum baseline, thinking module will no longer work with older Kotlin cores. And so it's a balancing act: at some point we will drop support for older cores, too, of course; but until then figuring out settings that are balanced. Having said that, I will let module maintainers -- specifically, @k163377 and @dinomite -- decide on what to do here, wrt version 2.15. |
|
Several verifications and considerations did not change the conclusion that it should not be merged. I also found a problem with this approach. Here is the verification code. verification code<?xml version="1.0" encoding="UTF-8"?>
<project xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="http://maven.apache.org/POM/4.0.0"
xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<artifactId>versioning-sandbox</artifactId>
<groupId>org.wrongwrong</groupId>
<version>1.0-SNAPSHOT</version>
<packaging>jar</packaging>
<name>consoleApp</name>
<properties>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<kotlin.code.style>official</kotlin.code.style>
<kotlin.compiler.jvmTarget>1.8</kotlin.compiler.jvmTarget>
<version.kotlin>1.4.32</version.kotlin>
<kotlin.compiler.languageVersion>1.4</kotlin.compiler.languageVersion>
<kotlin.compiler.apiVersion>1.4</kotlin.compiler.apiVersion>
</properties>
<repositories>
<repository>
<id>mavenCentral</id>
<url>https://repo1.maven.org/maven2/</url>
</repository>
</repositories>
<build>
<sourceDirectory>src/main/kotlin</sourceDirectory>
<testSourceDirectory>src/test/kotlin</testSourceDirectory>
<plugins>
<plugin>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-maven-plugin</artifactId>
<version>${version.kotlin}</version>
<executions>
<execution>
<id>compile</id>
<phase>compile</phase>
<goals>
<goal>compile</goal>
</goals>
</execution>
<execution>
<id>test-compile</id>
<phase>test-compile</phase>
<goals>
<goal>test-compile</goal>
</goals>
</execution>
</executions>
</plugin>
</plugins>
</build>
<dependencies>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-stdlib-jdk8</artifactId>
<version>${version.kotlin}</version>
</dependency>
<dependency>
<groupId>org.jetbrains.kotlin</groupId>
<artifactId>kotlin-reflect</artifactId>
<version>${version.kotlin}</version>
</dependency>
<dependency>
<groupId>com.fasterxml.jackson.module</groupId>
<artifactId>jackson-module-kotlin</artifactId>
<version>2.13.2</version>
</dependency>
</dependencies>
</project>package org.wrongwrong
import com.fasterxml.jackson.module.kotlin.jacksonObjectMapper
class Temp(val value: String)
fun main() {
val mapper = jacksonObjectMapper()
mapper.writeValueAsString(Temp("aaa"))
}There are no dependencies defined that would satisfy this condition, and there are CI grid tests, so this problem is unlikely to actually occur with the current version of |
|
Thanks for taking the time to consider this. |
|
Thanks for the suggestion. |
1.5.32 -> 1.7.22.
1.8 is available as of Dec 28, 2022, but it was not immediately clear to me if this would be compatible.
This doesn't really count as code, so I don't see a need to fill out the Contributor License Agreement, if this gets merged.