Migrate away from Reload4j
Closed this issue · 6 comments
Reload4j is the continuation of Log4j 1 for those affected by log4shell.
It would be better to use a more recent version of an Slf4j binding such as logback, log4j2, or slf4j-simple.
Matter of fact, defining an slf4j binding in a library is a bit of an anti-pattern. Consumers such applications do require explicit bindings, libraries do not.
However, a binding may be needed for testing. Thus changing the scope of the logging binding to test would be better.
IF the logging binding is indeed intended for just testing I'd suggest going with slf4j-simple as it's the easiest (no more deps) to use.
Reading through your amazing modular dependency tracker, we would essentially need to replace the log4j.properties file with a simplelogger.properties with the configuration for the project. Then also updating the pom dependencies.
Found this issue very interesting and just wanted to learn about the requirements behind it!
The thing is, that log4j.properties file should not even be at src/main/resources as the consumer may use a completely different slf4j binding, rendering that file useless.
However, it does make sense providing a logging binding for tests, which is why I suggest choosing either logback or slf4j-simple, but slf4j-log4j may remain as the chosen binding. Besides updating the <scope> of the slf4j-log4j property to test we'd also need to move log4j.properties from src/main/resources to src/test/resources.
@afrittoli I'm waiting for consensus/guidance before sending a PR. As I see it given my experience with Slf4j, cdevents-java should only define slf4j-api as part of its compile scope. slf4j-simple may be used for tests.
I can send a PR pretty soon if there's agreement on how to handle logging dependencies.
@aalmiray yes the existing logging is just used to print in the console when testing, I think you can create a PR as per your suggestion.
🎉 This issue has been resolved in v0.1.2 (Release Notes)