Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate implicit materialization of Slf4jFactory #683

Merged
merged 6 commits into from
Sep 20, 2022
Merged

Deprecate implicit materialization of Slf4jFactory #683

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d00181c
Deprecate implicit use of `LoggerFactory`
armanbilge Aug 23, 2022
c515035
Revert "Deprecate implicit use of `LoggerFactory`"
armanbilge Aug 23, 2022
64ce5e5
Only deprecated `loggerFactoryforSync`
armanbilge Aug 23, 2022
fe1883a
Update docs
armanbilge Aug 23, 2022
6c60b66
Fix typo
armanbilge Aug 23, 2022
93aa9ba
bump deprecation version
armanbilge Sep 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/shared/src/main/scala/org/typelevel/log4cats/LoggerFactoryGen.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,16 +27,22 @@ trait LoggerFactoryGen[F[_]] {
}

private[log4cats] trait LoggerFactoryGenCompanion {
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def getLogger[F[_]](implicit lf: LoggerFactoryGen[F], name: LoggerName): lf.LoggerType =
lf.getLogger
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def getLoggerFromName[F[_]](name: String)(implicit lf: LoggerFactoryGen[F]): lf.LoggerType =
lf.getLoggerFromName(name)
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def getLoggerFromClass[F[_]](clazz: Class[_])(implicit lf: LoggerFactoryGen[F]): lf.LoggerType =
lf.getLoggerFromClass(clazz)
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def create[F[_]](implicit lf: LoggerFactoryGen[F], name: LoggerName): F[lf.LoggerType] =
lf.create
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def fromName[F[_]](name: String)(implicit lf: LoggerFactoryGen[F]): F[lf.LoggerType] =
lf.fromName(name)
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def fromClass[F[_]](clazz: Class[_])(implicit lf: LoggerFactoryGen[F]): F[lf.LoggerType] =
lf.fromClass(clazz)
}
16 changes: 16 additions & 0 deletions slf4j/src/main/scala/org/typelevel/log4cats/slf4j/Slf4jFactory.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
package org.typelevel.log4cats
package slf4j

import cats.effect.kernel.Sync
import org.slf4j.{Logger => JLogger}

trait Slf4jFactory[F[_]] extends LoggerFactory[F] {
Expand All @@ -26,8 +27,23 @@ trait Slf4jFactory[F[_]] extends LoggerFactory[F] {
}

object Slf4jFactory extends LoggerFactoryGenCompanion {
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
def apply[F[_]: Slf4jFactory]: Slf4jFactory[F] = implicitly

def create[F[_]: Sync]: Slf4jFactory[F] = new Slf4jFactory[F] {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would have been more convenient as an apply method, but unfortunately that's been squatted :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's good this way! cats.effect.std.Console is also instatiated using make instead of apply. Maybe we should rename this to make rather than create :D

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, agree now that we decided we are not totally deprecating implicit (otherwise, the apply was dead deprecated weight).

Regarding naming: I chose create based on this.

log4cats/slf4j/src/main/scala/org/typelevel/log4cats/slf4j/Slf4jLogger.scala

Line 38 in 4f02ef3

def create[F[_]: Sync](implicit name: LoggerName): F[SelfAwareStructuredLogger[F]] =

But I think make is good too 🤔 and maybe better to keep it independent of create.

override def getLoggerFromName(name: String): SelfAwareStructuredLogger[F] =
Slf4jLogger.getLoggerFromName(name)

override def getLoggerFromSlf4j(logger: JLogger): SelfAwareStructuredLogger[F] =
Slf4jLogger.getLoggerFromSlf4j(logger)

override def fromName(name: String): F[SelfAwareStructuredLogger[F]] =
Slf4jLogger.fromName(name)

override def fromSlf4j(logger: JLogger): F[SelfAwareStructuredLogger[F]] =
Slf4jLogger.fromSlf4j(logger)
}

def getLoggerFromSlf4j[F[_]](logger: JLogger)(implicit
lf: Slf4jFactory[F]
): SelfAwareStructuredLogger[F] = lf.getLoggerFromSlf4j(logger)
Expand Down
16 changes: 3 additions & 13 deletions slf4j/src/main/scala/org/typelevel/log4cats/slf4j/package.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,10 @@
package org.typelevel.log4cats

import cats.effect.Sync
import org.slf4j.{Logger => JLogger}

package object slf4j {
implicit def loggerFactoryforSync[F[_]: Sync]: Slf4jFactory[F] = new Slf4jFactory[F] {
override def getLoggerFromName(name: String): SelfAwareStructuredLogger[F] =
Slf4jLogger.getLoggerFromName(name)

override def getLoggerFromSlf4j(logger: JLogger): SelfAwareStructuredLogger[F] =
Slf4jLogger.getLoggerFromSlf4j(logger)

override def fromName(name: String): F[SelfAwareStructuredLogger[F]] =
Slf4jLogger.fromName(name)

override def fromSlf4j(logger: JLogger): F[SelfAwareStructuredLogger[F]] =
Slf4jLogger.fromSlf4j(logger)
}
@deprecated("LoggerFactories should be passed explicitly, not implicitly", "2.5.0")
implicit def loggerFactoryforSync[F[_]](implicit F: Sync[F]): Slf4jFactory[F] =
Slf4jFactory.create(F)
}
2 changes: 1 addition & 1 deletion slf4j/src/test/scala/org/typelevel/log4cats/slf4j/Slf4jLoggerMacroCompilationTests.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ class Slf4jLoggerOuterClassMacroTest {

object LoggingBaseline {
val t = new Throwable
def logger[F[_]: Sync]: SelfAwareStructuredLogger[F] = Slf4jFactory.getLogger[F]
def logger[F[_]: Sync]: SelfAwareStructuredLogger[F] = Slf4jFactory.create[F].getLogger

val traceM = logger[IO].trace("")
val traceTM = logger[IO].trace(t)("")
Expand Down