Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace LOG4J12 with LOG4J2 #43

Closed
bbottema opened this issue Jun 14, 2016 · 6 comments
Closed

Replace LOG4J12 with LOG4J2 #43

bbottema opened this issue Jun 14, 2016 · 6 comments
Assignees
Milestone

Comments

@bbottema
Copy link
Owner

After moving to log4j12 in #14, let's modernize further and move to log4j2 as default logging implementation.

@bbottema bbottema added this to the 4.0.0 milestone Jun 14, 2016
@bbottema bbottema self-assigned this Jun 14, 2016
@zimmi
Copy link

zimmi commented Jun 15, 2016

Please reconsider adding a dependency on a concrete slf4j implementation. In my opinion, libraries better depend only on the api alone (and all the ones I know do so, I can provide examples if you want).

The current approach only works without configuration when the user:

  • has no binding to an implementation (and is fine with the defaults of log4j2)
  • is already using the same version of log4j2, in which case it doesn't help

In all other scenarios it introduces version or binding conflicts and users have to hunt down the reason.

If there is no binding to an implementation, slf4j will print a warning. If users want logging output, they should know what concrete logging system they are using and how to configure it.

@bbottema
Copy link
Owner Author

bbottema commented Jun 16, 2016

@zimmi I was thinking of providing documentation to exclude the log4j2 implementation from the maven dependencies so you can switch to another.

The API that is used in the code already is that of SLF4J.

However, if using the basic SLF4J implementation logging requires no further configuration from the user if they want to switch implementation, that would be preferable. I only included some implementation so that the demo and test classes produce output.

@bbottema bbottema reopened this Jun 16, 2016
@zimmi
Copy link

zimmi commented Jun 16, 2016

What do you think about instead providing documentation to add log4j2 as the recommended modern logging implementation?

Adding a link to the slf4j manual for the demo code for users that want logging output might be enough, maybe.

@bbottema
Copy link
Owner Author

bbottema commented Jun 17, 2016

I added it as an optional dependency using the maven <scope>optional</scope>. This way it is included during development, but won't be used by depending projects.

bbottema added a commit that referenced this issue Jun 17, 2016
bbottema added a commit that referenced this issue Jun 17, 2016
@bbottema
Copy link
Owner Author

I also moved the log4j config file to the test resources folder now so that won't be packaged either (see #45).

@zimmi
Copy link

zimmi commented Jun 27, 2016

I agree with that decision. :)
From a user standpoint, I just want to send mail. The fewer dependencies and assumptions a library makes, the better. Opinions are for frameworks.
Thanks again for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants