Skip to content

Conversation

@dongxuwang
Copy link
Contributor

References #31395

This would significantly enlarge the change to get slf4j warning silent

@dongxuwang dongxuwang changed the title have a tiny buffer for slf4j to get initialized #31395 remove info log showing Slf4jLogger started to avoid slf4j warning at the top #31395 Aug 15, 2023
@dongxuwang
Copy link
Contributor Author

Hi @patriknw , I updated the pr, do you think we also need to un-extend Slf4jLogger with SLF4JLogging?

}

case InitializeLogger(_) =>
log.info("Slf4jLogger started")
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should anyway touch the log because that is a lazy val and if we don't touch it we haven't really started the intiialization yet, and next real log message will do that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about let Slf4jLogger NOT to extend SLF4JLogging

Copy link
Contributor

Choose a reason for hiding this comment

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

That would be fine, but it would still have to create the logger at some point.

Well, there is a problem with compatibility if removing SLF4JLogging, because Slf4jLogger is public api and not final. Someone may have extended it and expect that log.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, that's also another point.

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