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

Consider switching logging to slf4j #9

Open
badgerwithagun opened this issue Jun 10, 2017 · 10 comments
Open

Consider switching logging to slf4j #9

badgerwithagun opened this issue Jun 10, 2017 · 10 comments

Comments

@badgerwithagun
Copy link
Member

Currently we force a log4j dependency on applications. There's no need.

@badgerwithagun badgerwithagun added this to the 1.0.0 milestone Jun 10, 2017
@badgerwithagun
Copy link
Member Author

@venushka - would value your thoughts on this one.

@tsg21
Copy link
Member

tsg21 commented Jul 3, 2017

Yes, I think this is a sensible plan.

Currently we force a log4j dependency on applications. There's no need.

We don't - commons.logging can be hooked into other logging implementations, but it's nowhere near as widely used as slf4j is.

@venushka
Copy link
Member

venushka commented Jul 3, 2017

I agree, we should switch to slf4j.

One thing to keep in mind though, we should make sure the test execution within Maven should be provided with a logging implementation (log4j like we already have probably) with the required configuration to ensure the logs from tests are redirected to a file and not be printed to console. Travis builds are limited to 4MB of console output for opensource projects, which is more than adequate for the build, but not including tests logs in console.

@badgerwithagun
Copy link
Member Author

Yeah I saw that on your fork - makes sense.

@tsg21
Copy link
Member

tsg21 commented Oct 27, 2017

I've had a go at this in a fork. It's a reasonably easy find-replace.

@badgerwithagun
Copy link
Member Author

badgerwithagun commented Oct 27, 2017

@tsg21 - submit a PR!

Actually... scratch that. Let's confirm we actually want to do it first. If so, it'll be easier to just push directly.

@tsg21
Copy link
Member

tsg21 commented Oct 27, 2017

Don't see any reason why we'd want to stick to commons-logging?

@tsg21
Copy link
Member

tsg21 commented Nov 22, 2017

Anyone like to offer up a reason why we should keep the dependency on log4j?

@badgerwithagun
Copy link
Member Author

Nope. Let's just do this. Could you create a PR with commons->slf4j and removal of log4j?

@tsg21
Copy link
Member

tsg21 commented Nov 23, 2017

Will do

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

3 participants