Add LoggerFactory typeclass #629
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The reasoning for this commit is that I want to avoid having a Sync instance just for being able to log in a http4s api. Extract LoggerName from macro and make it an implicit. This makes it easier to have forwarders to Slf4jLogger. This should be binary compatible, I had to add some exclusions to mima because of the missing macro methods.
hamnis
commented
Mar 24, 2022
slf4j/src/main/scala-2/org/typelevel/log4cats/slf4j/Slf4jLogger.scala
Outdated
Show resolved
Hide resolved
|
This also resolves #492. I can say that there are uncounted requests on it from TF-folks. So it's nice to see this PR 👍🏻 |
FunFunFine
reviewed
Mar 24, 2022
slf4j/src/main/scala-2/org/typelevel/log4cats/slf4j/Slf4jLogger.scala
Outdated
Show resolved
Hide resolved
FunFunFine
reviewed
Mar 24, 2022
slf4j/src/main/scala-2/org/typelevel/log4cats/slf4j/Slf4jLoggerFactory.scala
Outdated
Show resolved
Hide resolved
FunFunFine
reviewed
Mar 24, 2022
slf4j/src/main/scala-2/org/typelevel/log4cats/slf4j/internal/GetLoggerMacros.scala
Outdated
Show resolved
Hide resolved
FunFunFine
reviewed
Mar 24, 2022
slf4j/src/main/scala-3/org/typelevel/log4cats/slf4j/Slf4jLoggerFactory.scala
Outdated
Show resolved
Hide resolved
FunFunFine
reviewed
Mar 24, 2022
slf4j/src/main/scala-3/org/typelevel/log4cats/slf4j/Slf4jLoggerFactory.scala
Outdated
Show resolved
Hide resolved
* Extract Platform traits for different code * LoggerName moved to shared * LoggerFactory with convenience methods.
Closed
4 tasks
slf4j/src/main/scala/org/typelevel/log4cats/slf4j/LoggerFactory.scala
Outdated
Show resolved
Hide resolved
danicheg
reviewed
Apr 5, 2022
slf4j/src/main/scala-2/org/typelevel/log4cats/slf4j/LoggerNamePlatform.scala
Outdated
Show resolved
Hide resolved
slf4j/src/test/scala/org/typelevel/log4cats/slf4j/LoggerNameTest.scala
Outdated
Show resolved
Hide resolved
slf4j/src/main/scala/org/typelevel/log4cats/slf4j/LoggerName.scala
Outdated
Show resolved
Hide resolved
* Extracted LoggerFactory and LoggerFactoryGen * Moved LoggerName to core and added macro there * Renamed Platform traits to Compat traits * Moved Slf4jLoggerFactory into Sl4j4Logger.Factory trait
hamnis
commented
Apr 5, 2022
slf4j/src/main/scala/org/typelevel/log4cats/slf4j/Slf4jLogger.scala
Outdated
Show resolved
Hide resolved
This reverts commit 7167e54.
hamnis
commented
Apr 6, 2022
| trait LoggerFactory[F[_]] extends LoggerFactoryGen[F, SelfAwareStructuredLogger[F]] | ||
| object LoggerFactory extends LoggerFactoryCompanion { | ||
| def apply[F[_]: LoggerFactory]: LoggerFactory[F] = implicitly | ||
| trait LoggerFactory[F[_]] extends LoggerFactoryGen[F] { |
There was a problem hiding this comment.
we can consider dropping this, since LoggerFactoryGen now has everything that is needed.
There was a problem hiding this comment.
It's kinda convinient for users who would like to use just LoggerFactory without the need to define LoggerType explicitly
|
Any chance this can get merged? @ChristopherDavenport ? |
ChristopherDavenport
approved these changes
Apr 16, 2022
|
That is correct. There are no mima issues or exclusions here |
|
This seems awesome, thanks for the hardwork! I definitely wouldn't want to block this, but is there any chance a bit of documentation could be added, or an example of how one might use this? |
lorandszakacs
approved these changes
Apr 20, 2022
|
Going to roll this out tomorrow. Really awesome work. |
This was referenced Jun 2, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The reasoning for this PR is that I want to avoid having a Sync instance just for being able to log in a http4s api.
Extract LoggerName from macro and make it an implicit.
This makes it easier to have forwarders to Slf4jLogger.
This should be binary compatible.