search

oshai / kotlin-logging

Lightweight Multiplatform logging framework for Kotlin. A convenient and performant logging facade.
Other
2.66k stars 112 forks source link

Logback backend for kotlin-logging #452

Closed neeme-praks-sympower closed 3 weeks ago

neeme-praks-sympower commented 1 month ago

Proposal for https://github.com/oshai/kotlin-logging/issues/449 I'm not sure if this change requires kotlin-logging to always have Logback on runtime classpath or not (how eager is JVM classloading nowadays?) -- but we can figure that detail out later.

oshai commented 1 month ago

Right now it fails on formatting, see how to fix spotless here: https://github.com/oshai/kotlin-logging/blob/master/CONTRIBUTING.md

In addition, since I want it to be both extended (with <class>, <method>, <line>, <file>) maybe we should wrap it with compilerOptions or internalCompilerData data class. I think it will be more obvious that this should not be set.

neeme-praks-sympower commented 1 month ago

Thanks for the feedback! I'll get back to this next week.

neeme-praks-sympower commented 1 month ago

I've now fixed formatting issues and introduced internalCompilerData field.

oshai commented 3 weeks ago

Can you please fix spotless again before I submit it?

neeme-praks-sympower commented 3 weeks ago

Can you please fix spotless again before I submit it?

Fixed.

I would like to also include a test that would execute JulLoggerWrapperTest without Logback in the classpath -- to make sure that KLoggerFactory can access JulLoggerFactory without needing to load LogbackLoggerFactory (which depends on Logback).

I tried to play around with Gradle build file but my knowledge about Kotlin multiplatform (and Gradle) build system is limited -- maybe you can help out here?

oshai commented 3 weeks ago

Can you please fix spotless again before I submit it?

Fixed.

I would like to also include a test that would execute JulLoggerWrapperTest without Logback in the classpath -- to make sure that KLoggerFactory can access JulLoggerFactory without needing to load LogbackLoggerFactory (which depends on Logback).

I tried to play around with Gradle build file but my knowledge about Kotlin multiplatform (and Gradle) build system is limited -- maybe you can help out here?

Thanks! I merged the PR and just now see this message. I think it will be difficult to do tests with / without logback in classpath. I also don't think gradle is very good at this. I think we can go without such test.

neeme-praks-sympower commented 3 weeks ago

I think we can go without such test.

Just to be sure, I ran a manual test with a simple project on my machine -- seems to work just fine (using -Dkotlin-logging-to-jul=true and without Logback in the classpath).

Thanks!