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.

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

Suggestion for changing Logging hierarchy #1268

Open
geny200 opened this issue May 6, 2024 · 2 comments
Open

Suggestion for changing Logging hierarchy #1268

geny200 opened this issue May 6, 2024 · 2 comments

Comments

@geny200
Copy link
Contributor

geny200 commented May 6, 2024

I looked at the hierarchy of Logging and ServiceLogging, and I don't realize why Logging extends from ServiceLogging, not the other way round. I suggest swapping them, also removing the method def to[Svc2]: ServiceLogging[F, Svc2] (because this method does not realy change the name of the logger).

suggestion:

trait Logging[F[_]] extends LoggingBase[F] {
  final def widen[G[a] >: F[a]]: Logging[G] = this.asInstanceOf[Logging[G]]

  def asLogging: Logging[F] = this
}

trait ServiceLogging[F[_], Service] extends Logging[F] {
  @deprecated
  final def to[Svc2]: ServiceLogging[F, Svc2] = this.asInstanceOf[ServiceLogging[F, Svc2]]
}

also change logs factory:

trait Logs[+I[_], F[_]] extends LogsVOps[I, F] {
  // Here is the difference
  def forService[Svc](implicit Svc: ClassTag[Svc]): I[ServiceLogging[F, Svc]] = 
    byName(Svc.runtimeClass.getName()).asInstanceOf[I[ServiceLogging[F, Svc]]]

  // Here is the difference
  def byName(name: String): I[ServiceLogging[F, Nothing]]

  final def biwiden[I1[a] >: I[a], F1[a] >: F[a]]: Logs[I1, F1] = this.asInstanceOf[Logs[I1, F1]]

  final def service[Svc: ClassTag]: I[ServiceLogging[F, Svc]] = forService[Svc]

  final def of[Svc[_[_]]](implicit tag: ClassTag[Svc[HKAny]]): I[ServiceLogging[F, Svc[HKAny]]] =
    service[Svc[HKAny]]
}

The reason why I propose this change is because now the base class for logging is ServiceLogging[F, Svc] (base LoggingBase is deprecated), so to add correct syntax impl for logging, you need to require implicit ServiceLogging[F, Any], not Logging[F] which is not obvious.

@dos65
Copy link
Collaborator

dos65 commented May 14, 2024

@geny200 Thanks for proposal. However I would avoid making such changes as it will add compat issues.

The reason why I propose this change is because now the base class for logging is ServiceLogging[F, Svc] (base LoggingBase is deprecated), so to add correct syntax impl for logging, you need to require implicit ServiceLogging[F, Any], not Logging[F] which is not obvious.

I think that introducing incompatible changes will bong more problems that and issue you describe in this motivation. wdyt?

@geny200
Copy link
Contributor Author

geny200 commented May 14, 2024

I think that introducing incompatible changes will bong more problems

The only incompatible change here will be that the Logging type will no longer have the "to" method (I consider that this method should not exist in its current form, because it provides a cheat for types), so we can add "to" method for binary compatibility to Logging and immediately set as deprecated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants