qos-ch / slf4j

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

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

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.

m-froehlich commented 1 year ago

Hi community, we had problems with a library, that is using SLF4j, that is loaded through a custom ClassLoader to workaround service provider initialization issues. For this to work, we need to make sure, that service providers are looked up in the correct class loader. My changes should do that without any drawbacks to the current behavior.

Unfortunately changes were made to this exact area in the code. So I had to adjust my patch a little. But it should do the trick now.

m-froehlich commented 1 year ago

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

m-froehlich commented 1 year ago

Do I have to do anything else to workaround the signed-off-by thing?

HannesWell commented 1 year ago

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

You have to add that to the bottom of your commit message. If you are using Eclipse, the IDE can add that for you if you enable it in the Git Staging View's Commit Message area, based on your git-config (the pencil symbol).

m-froehlich commented 1 year ago

I have committed it using this github platform. Is it ok, if I commit something arbitrary (a space or something) and add it to that commit message?

HannesWell commented 1 year ago

I have committed it using this github platform. Is it ok, if I commit something arbitrary (a space or something) and add it to that commit message?

I'm not a maintainer of this library so I cannot tell what is really desired, but since each commit has to be signed off (i.e. has the Signed-off-by footer), I would simply amend your first commit, remove the second one and force-push to this branch again.

ceki commented 1 year ago

This PR is not backward compatible and cannot be merged as is.

m-froehlich commented 1 year ago

I had already deleted this branch. But what about the patch-2 branch? Have you seen that PR?