binance-connector-java
binance-connector-java copied to clipboard
Remove import of logging library
Libraries should not import both SLF4J and a logger. Doing so voids the purpose of a common logging framework completely, since it forces a logging library on the end-user rather than leaving the ability to choose one.
From the SLF4J's official FAQs:
Embedded components such as libraries not only do not need to configure the underlying logging framework, they really should not do so. They should invoke SLF4J to log but should let the end-user configure the logging environment. When embedded components try to configure logging on their own, they often override the end-user's wishes. At the end of the day, it is the end-user who has to read the logs and process them. She should be the person to decide how she wants her logging configured.
[APPROVALNOTIFIER] This PR is NOT APPROVED
This pull-request has been approved by: andrea-amadei
The full list of commands accepted by this bot can be found here.
Approvers can indicate their approval by writing /approve
in a comment
Approvers can cancel approval by writing /approve cancel
in a comment
You can configure your log settings under resources -> logback.xml
You can configure your log settings under resources -> logback.xml
What about if I don’t want to use Logback? And what about if I don’t want to use any logger at all?
The purpose of SLF4J is exactly this - to provide a wrapper for logging, but to also let the end-user itself choose it. Since this project is intended as a library, including both Logback and SLF4J (or any other logger from the SLF4J family) completely voids the effectiveness of the latter.
Here is in practical terms what is happening in the current state:
- The user doesn’t need a logger -> the logger is forced upon him anyways
- The user wants a logger but not Logback -> the user will be stuck with both loggers since Maven will include both. He will also need two config files at the same time
- The user wants to use Logback -> everything works as intended, but at this point SLF4J isn’t needed anymore, since anything could be achieved with Logback anyways.
The only practical way to fix this is to remove Logback from the Maven dependencies (and any other reference to it left in the code, although if SLF4J was used correctly there shouldn’t be any).
I've also pushed a commit to remove any further logback reference in the code. Everything should be good to go!
If you remove the logback dependencies, does the project compile? SLF4J requires an underlying logging framework in order for it to work.
If you remove the logback dependencies, does the project compile?
It does, since SLF4J injects the logger at runtime. The only side-effect is a message printed in console, warning that SLF4J is running without a logger loaded in. It shouldn't be a problem since the final user might only see it if he is not importing a logger either.
This error is thrown when i try to run an example using your PR, can you try it?
This is the warning message I was talking about.
It is absolutely normal and won’t disrupt execution by any means, since it’s not an Exception nor an interrupting error. Its purpose is to warn the final user no logger was found; therefore, any logging operation won’t take place. As a matter of facts NOP means “no operation” since no logging operation will take place. Because you are trying to build a Java library instead of an end-user application, you can simply ignore that message; it’s not directed to you.
While importing this library, the end-user will be able to pick any logger with SLF4J support, and they will never encounter a single problem. Only if they do not wish to use a logger, the warning message will appear, but everything will work fine. If the end user really wish to get rid of the warning message without picking a specific logging environment, they should import slf4j-nop instead. It's a logging environment that does literally nothing, only suppressing the previous warning message.
Check it out for yourself here and here:
"If you are responsible for packaging an application and do not care about logging, then placing slf4j-nop.jar on the class path of your application will get rid of this warning message."
"Note that embedded components such as libraries or frameworks should not declare a dependency on any SLF4J binding but only depend on slf4j-api. When a library declares a compile-time dependency on a SLF4J binding, it imposes that binding on the end-user, thus negating SLF4J's purpose."
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.
@andrea-amadei: PR needs rebase.
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.