search

qos-ch / slf4j

Simple Logging Facade for Java
http://www.slf4j.org
MIT License
2.32k stars 981 forks source link

Added option to load service provider from passed in ClassLoader #336

Closed m-froehlich closed 1 year ago

m-froehlich commented 1 year ago

The LoggerFactory will load service providers from a passed in ClassLoader, if given. Otherwise previous behavior is maintained. The getLogger(Class) method receives a class, that is used to extract a ClassLoader from.

Signed-off-by: Marvin Fröhlich m.froehlich@infolog.de

m-froehlich commented 1 year ago

I have created another patch branch with the same changes, but with Singed-off-by line in the commit message.

What checks haved failed? What do I need to do?

ceki commented 1 year ago

This change introduces behavior which is subtly different than older code. It will not be merged.

m-froehlich commented 1 year ago

I am very confused. Does my code break anything? Does it not give you more options? Does it not fix a standing issue?

What is the exact reason for not merging it into the master?

PAC4J had the same problem. And they did accept my patch.

m-froehlich commented 1 year ago

Without my patch slf4j simply doesn't work in combination with a custom class loader.

m-froehlich commented 1 year ago

Can you please comment on this? This really is a bug in the code, that needs to be fixed.

Do I have to do anything to make this fix good to be merged?

ceki commented 1 year ago

Your patch changes the public API. Moreover, it modifies existing behavior and has a much bigger impact than you seem to think.

It will not be merged. I can't be clearer than this.

m-froehlich commented 1 year ago

Are you open to an API extension on that front, that can be used optionally to workaround this bug? Hopefully the guys at PAC4J will make use of it then.