search

qos-ch / slf4j

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

Make `slf4j.provider` constant public. #362

Closed garretwilson closed 9 months ago

garretwilson commented 10 months ago

Good work @KengoTODA on SLF4J-450. Unfortunately the PROVIDER_PROPERTY_KEY constant is not public in LoggerFactory:

static final public String PROVIDER_PROPERTY_KEY = "slf4j.provider";

This is part of the official, documented API, so it's appropriate that it should be public. After all I might want to programmatically set the system property like this (see #361), for example in JUnit tests:

@BeforeAll
static void disableLogging() {
  System.setProperty("slf4j.provider", NOP_FallbackServiceProvider.class.getName());
}

If nobody has any objections, I can file a pull request for this.

ceki commented 10 months ago

@garretwilson No objections and I agree with your assessment. However, PROVIDER_PROPERTY_KEY is already public as can be seen in commit 2481810f94f32923db5eb86f. Am I missing something?

garretwilson commented 10 months ago

Huh, I don't know how I missed that. Maybe it was because I was just looking at the source code and was accustomed to see public as the first modifier. (See discussion on order at https://stackoverflow.com/a/16732059 .)

Let me try it again in my IDE …

garretwilson commented 10 months ago

Well, duh, I even showed the source code in the description!! 🤦‍♂️

It was toward the end of the day, so I think I just missed it because I'm used to seeing public as the first modifier. And when I tried it in the IDE I must have accidentally used ILoggerFactory instead of LoggerFactory.

It seems I was mistaken. You can close this ticket as invalid. Thanks!