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

Logger config #668

Merged
merged 53 commits into from
Mar 14, 2023
Merged

Logger config #668

merged 53 commits into from
Mar 14, 2023

Conversation

justcoon
Copy link
Contributor

@justcoon justcoon commented Feb 26, 2023

loggers configuration support

added LogPattern , which is string representation of LogFormat - zio-parser is used for parsing

removed scala 2.11 support

@jdegoes
Copy link
Member

jdegoes commented Feb 27, 2023

  1. Yes, I'd drop Scala 2.11.
  2. A class called X should contain constructors in companion object of X. The pattern can be broken if necessary, but it is always confusing to do so.

@justcoon justcoon marked this pull request as ready for review March 7, 2023 20:49
@justcoon justcoon requested a review from a team as a code owner March 7, 2023 20:49
/**
* A `LogPattern` is string representation of `LogFormat`
*/
sealed trait LogPattern {
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if we should call this SerializableLogFormat, or perhaps, LogFormat.Serializable or something.

The presence of toLogFormat suggests that it might be or should be a subtype of LogFormat.

Expressing that relationship in the name could be useful. Also moving it to the companion object of LogFormat could be useful if it's not as commonly used as LogFormat, and we want to hide it a bit or emphasize it's a derivative of LogFormat.

def name: String
}

sealed trait Arg1[A1] extends Arg {
Copy link
Member

Choose a reason for hiding this comment

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

The type parameters here become erased because they are not tracked in the supertype.

Which means this is practically equivalent to def arg1: Any, etc.

I wonder if, instead of doing it this way, we can either delete Arg1 and Arg2, or embed type-safe capabilities into these treats, which lets clients of the traits use the arguments in some safe way?

jdegoes
jdegoes previously approved these changes Mar 10, 2023
Copy link
Member

@jdegoes jdegoes left a comment

Choose a reason for hiding this comment

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

Beautiful

@jdegoes jdegoes merged commit d3ae5ce into zio:master Mar 14, 2023
@justcoon justcoon deleted the logger_config branch May 26, 2023 15:54
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

Successfully merging this pull request may close these issues.

2 participants