Factorize and cooler Logs #567
Conversation
Odomontois
requested review from
catostrophe,
danslapman,
oskin1 and
Z1kkurat
as code owners
higherKindCore/src/test/scala/tofu/higherKind/SyntaxCheck.scala
Outdated
Show resolved
Hide resolved
| import derevo.PassTypeArgs | ||
| import derevo.ParamRequire | ||
|
|
||
| object loggingMid |
There was a problem hiding this comment.
We definitely need some scaladoc here, probably with the link to the docs.
Those are things that have to be described:
- differences between all of
loggingMid* - how each method will be logged
- how to configure logging
- where the
Logginginstance will be looked for
There was a problem hiding this comment.
Added docs describing the first three.
Remove className from default logger
Odomontois
force-pushed
the
factorize
branch
2 times, most recently
from
0c316ac
to
ce99b57
Compare
There was a problem hiding this comment.
V E L I C H I E is as close as it never was.
One side note: I think the segregation between unsafe impure code has to be more clear. Tofu users should easily see that some code doesn't break RT or, if they need to break it, it should be clear too. Something like import tofu.logging.unsafe.
When there's no such difference it can lead to false assumptions or annoying bugs.
Although this can be discussed in the background and be done later.
| @@ -0,0 +1,3 @@ | |||
| trait Ingredient { | |||
There was a problem hiding this comment.
I couldn't find any usages of this type. Perhaps it should be removed
logging/derivation/src/main/scala/tofu/logging/derivation/loggingMid.scala
Outdated
Show resolved
Hide resolved
logging/derivation/src/main/scala/tofu/logging/derivation/loggingMid.scala
Outdated
Show resolved
Hide resolved
logging/derivation/src/main/scala/tofu/logging/derivation/loggingMid.scala
Outdated
Show resolved
Hide resolved
| args: mutable.Buffer[(String, LoggedValue)] | ||
| ) { | ||
| def arg[A: Loggable](name: String, a: A): this.type = { | ||
| args += (name -> a) |
There was a problem hiding this comment.
Shouldn't this code be marked as unsafe somehow with comments or something?
The reader would need to understand the intentions behind the decision to make it mutable and such.
There was a problem hiding this comment.
I suppose we can split this unsafe behavior into the package-private implementation of the abstract class, as OOP grandfather behest says
| } | ||
|
|
||
| /** Logging middleware generator */ | ||
| trait LoggingMidBuilder { |
There was a problem hiding this comment.
Somebody once told me that mixing up "utility" code like all those bulders and mutable stuff together with actual API hurts readability and discoverability.
One can almost sink in all this stuff.
Is it possible to hide it in some folders or in other files to keep things clean?
WDYT?
There was a problem hiding this comment.
This is not a utility code.
Currently I have a habit of hiding utility things in the impl package.
But this one is something like a configuration as a code interface, one can easily craft its own logging mid derivation using this builder.
| object UniversalLogging { | ||
| final def enabled(level: Logging.Level, logger: Logger): Boolean = level match { | ||
| case Trace => logger.isTraceEnabled() | ||
| case Debug => logger.isDebugEnabled() | ||
| case Info => logger.isInfoEnabled() | ||
| case Warn => logger.isWarnEnabled() | ||
| case Error => logger.isErrorEnabled() | ||
| } | ||
|
|
||
| final def write(level: Logging.Level, logger: Logger, message: String, values: Seq[LoggedValue]): Unit = level match { | ||
| case Trace => logger.trace(message, values: _*) | ||
| case Debug => logger.debug(message, values: _*) | ||
| case Info => logger.info(message, values: _*) | ||
| case Warn => logger.warn(message, values: _*) | ||
| case Error => logger.error(message, values: _*) | ||
| } | ||
| final def writeMarker( | ||
| level: Logging.Level, | ||
| logger: Logger, | ||
| marker: Marker, | ||
| message: String, | ||
| values: Seq[LoggedValue] | ||
| ): Unit = | ||
| level match { | ||
| case Trace => logger.trace(marker, message, values: _*) | ||
| case Debug => logger.debug(marker, message, values: _*) | ||
| case Info => logger.info(marker, message, values: _*) | ||
| case Warn => logger.warn(marker, message, values: _*) |
There was a problem hiding this comment.
Those are impure as far as I see, so they should be marked as such somehow or even made private. I am like 99% sure that none of the tofu users will want to use the unsafe logger.
| def monoidK[X]: MonoidK[F[X, *]] = new MonoidK[F[X, *]] { | ||
| def combineK[A](x: F[X, A], y: F[X, A]): F[X, A] = combinebk(x, y) | ||
|
|
||
| def empty[A]: F[X, A] = emptybk | ||
| } | ||
|
|
||
| def leftMonoidK[X, Y]: MonoidK[F[*, X]] = new MonoidK[F[*, X]] { | ||
| def combineK[A](x: F[A, X], y: F[A, X]): F[A, X] = combinebk(x, y) | ||
|
|
||
| def empty[A]: F[A, X] = emptybk | ||
| } | ||
|
|
||
| def monoid[X, Y]: Monoid[F[X, Y]] = new Monoid[F[X, Y]] { | ||
| def combine(x: F[X, Y], y: F[X, Y]): F[X, Y] = combinebk(x, y) | ||
|
|
||
| def empty: F[X, Y] = emptybk | ||
| } |
There was a problem hiding this comment.
These ones might be implemented as vals + asInstanceOf, although it's not worth extra efforts.
| def semigroupK[X]: SemigroupK[F[X, *]] = new SemigroupK[F[X, *]] { | ||
| def combineK[A](x: F[X, A], y: F[X, A]): F[X, A] = combinebk(x, y) | ||
| } | ||
|
|
||
| def leftSemigroupK[X]: SemigroupK[F[*, X]] = new SemigroupK[F[*, X]] { | ||
| def combineK[A](x: F[A, X], y: F[A, X]): F[A, X] = combinebk(x, y) | ||
| } | ||
|
|
||
| def semigroup[X, Y]: Semigroup[F[X, Y]] = combinebk(_, _) |
Hide things away from sensitive eyes
…ingMid.scala Co-authored-by: Антон Войцишевский <31823086+FunFunFine@users.noreply.github.com>
Co-authored-by: Антон Войцишевский <31823086+FunFunFine@users.noreply.github.com>
No description provided.