You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Note: I know that's an old discussion, but just to revise it again.
Problem
Currently, the "idiomatic" way to create logger is a top-level variable.
importio.github.oshai.kotlinlogging.KotlinLogging// Place definition above class declaration, below imports, // to make field static and accessible only within the fileprivateval logger =KotlinLogging.logger {}
Lot's of JVM backend teams prefer to have private variables inside the class. What if we have couple of private static variables, should we also place them as a top level declaration? And what if we have instance private variables as well? This introduce inconsistency and hard-to-read code base, since some of the variables could be as a top-level, some of them as a companion object and some of them as a instance variables.
Exposes the top-level class to the public API.
If I'm writing a Java library, adding a variable at the top level introduces an public empty top-level class. In recent version of Kotlin we can "embed" the top-level variable using @file:JvmName
@file:JvmName("MyPublicAPI") // may work in recent kotlin versionspackageexampleimportio.github.oshai.kotlinlogging.KotlinLoggingprivateval logger =KotlinLogging.logger {}
classMyPublicAPI {
}
Top level variables introduce additional top level class (if we are not using @file:JvmName the same as class name). It just makes the final jar bigger.
There is ongoing proposal about static namespace/static variables and current syntax does not allow to migrate (with IDEA/compiler hint) from this syntax:
Suggested solution
My proposal would be to change logger(func: () -> Unit) to logger(ref: Any). Currently in all cases className derived via reflection (or even via stack trace in js), so it does not matter if it's a lambda or an object. The teams are free to chose what syntax to use then. And also the current code with lambda top-level declaration will work as well.
val topLevelNamedLogger =KotlinLogging.logger("TopLevelNamedLogger")
val topLevelLambdaLogger =KotlinLogging.logger {}
classMyClass {
val classNamedLogger =KotlinLogging.logger("MyClass")
val classLambdaLogger =KotlinLogging.logger {}
val classRefLogger =KotlinLogging.logger(this)
companionobject {
val companionNamedLogger =KotlinLogging.logger("MyClassCompanion")
val companionLambdaLogger =KotlinLogging.logger {}
val companionRefLogger =KotlinLogging.logger(this)
}
}
Braking changes
Logger property delegate for Kotlin/Js was removed. It's not possible to make it work in case of companion object.
KotlinLogging.logger(underlyingLogger: Logger) was removed due to the clash with object method.
Other changes
Covered all use cases for Kotlin/js and Kotlin/wasm
Optimize the KLoggerNameResolver for jvm, since we don't need to search indexOf multiple times
The text was updated successfully, but these errors were encountered:
Note: I know that's an old discussion, but just to revise it again.
Problem
Currently, the "idiomatic" way to create logger is a top-level variable.
If I'm writing a Java library, adding a variable at the top level introduces an public empty top-level class. In recent version of Kotlin we can "embed" the top-level variable using
@file:JvmName
Top level variables introduce additional top level class (if we are not using
@file:JvmName
the same as class name). It just makes the final jar bigger.Before Kotlin 2.0 each lambda functions compiled to the anonymous classes, that also makes final jar bigger, because for every logger we need a lambda. https://kotlinlang.org/docs/whatsnew20.html#generation-of-lambda-functions-using-invokedynamic
There is ongoing proposal about static namespace/static variables and current syntax does not allow to migrate (with IDEA/compiler hint) from this syntax:
to this
Suggested solution
My proposal would be to change
logger(func: () -> Unit)
tologger(ref: Any)
. Currently in all cases className derived via reflection (or even via stack trace in js), so it does not matter if it's a lambda or an object. The teams are free to chose what syntax to use then. And also the current code with lambda top-level declaration will work as well.Braking changes
Other changes
The text was updated successfully, but these errors were encountered: