Move logback to test scope #8
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Do not drag in a logging backend as a transitive dependency to client projects.
Owner
|
Sounds good! Debug logging was useful for development, but it definitely should not be the default logging level. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
It's great that slf4j is used, but it's not good that a library drags in a logging backend (logback-classic and logback-core artifacts) as a transitive dependency into projects using it. It creates bloat in the client project, might raise questions during security audits (why an unused logging framework is part of the project), etc.
Logback has also been upgraded to the latest version. I assumed this should not be a problem and did not bother separating scope change and version upgrade into separate commits. Let me know otherwise.
As an aside, I'm not a fan of DEBUG log level being enabled by default, which is the case for Logback when not configured explicitly. The debug log is useful when troubleshooting a failing test case, but when all test cases are passing (the typical case), debug logging just creates noise in build output possibly drowning out something more important. In a simple case like this I would opt for the more lightweight slf4j-simple instead of a full-blown logging framework, debug level could be enabled via a system property when actually needed. Alternatively, Logback configuration could be added to disable debug level by default. Either approach could be the subject of a separate PR, if desired.