-
Notifications
You must be signed in to change notification settings - Fork 74
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.
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
Helpers for creating simple/stub versions of SelfAwareStructuredLogger
and LoggerFactory
#682
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -28,6 +28,14 @@ trait ErrorLogger[F[_]] { | |
} | ||
|
||
object ErrorLogger { | ||
trait Fallback[F[_]] extends ErrorLogger[F] { _: MessageLogger[F] => | ||
final def error(t: Throwable)(message: => String): F[Unit] = error(message) | ||
final def warn(t: Throwable)(message: => String): F[Unit] = warn(message) | ||
final def info(t: Throwable)(message: => String): F[Unit] = info(message) | ||
final def debug(t: Throwable)(message: => String): F[Unit] = debug(message) | ||
final def trace(t: Throwable)(message: => String): F[Unit] = trace(message) | ||
Comment on lines
+32
to
+36
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I couldn't decide whether these should be final, and in fact wasn't consistent within this PR. Any thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think there's much reason to make these |
||
} | ||
|
||
def apply[F[_]](implicit ev: ErrorLogger[F]): ErrorLogger[F] = ev | ||
|
||
private def mapK[G[_], F[_]](f: G ~> F)(logger: ErrorLogger[G]): ErrorLogger[F] = | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm having a hard time understanding what the purpose of
Fallback[F[_]]
is.Is it a convenience mixin that ignores any
Throwable
s in the signatures ofErrorLogger[F[_]]
?Is it intended for end-users?
Seeing how it's used here I'd rather have it just be an implementation detail of the
SelfAwareStructuredLogger.liftF
because otherwise it increases the API surface area of the library quite a lot 😅There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. I feel like it's a useful thing to have available but I don't have a very strong, specific answer to your question so yes, probably best to leave it as an implementation detail for now at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer making all the helper traits
private[log4cats]
(or whatever is best for bincompat, I am really bad at this part 😬). We can always make them public later with little effort and in a minor version.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private[log4cats]
doesn't really protect bincompat, but it protects source compat, and we can grant a MiMa waiver if we're sure nothing else used it.In multi-repo projects like http4s, it's unsafe to waive away package privates, because usages could be in another repo. Here, it's probably safe: nothing other than this project should be in the
log4cats
namespace.Either way, I'd agree locking it down is safer for libraries this low in the hierarchy.