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

build: migrate logger to SLF4J APIs + Logback backend #513

Merged
merged 7 commits into from
Nov 27, 2024

Conversation

NiccoMlt
Copy link
Contributor

@NiccoMlt NiccoMlt commented Nov 8, 2024

  • migrate java.util.logging usages to SLF4J
  • drop Apache Commons Logging dependency
  • add Apache Commons Logging to SLF4J adapter because of other dependencies
  • drop JUL backend for SLF4J
  • add Logback as backend for SLF4J

The logger doesn't need the logging.properties file anymore.
The configuration logging.debug.enabled was dropped; just configure the log level in the logback.xml file instead

@NiccoMlt NiccoMlt linked an issue Nov 8, 2024 that may be closed by this pull request
@NiccoMlt NiccoMlt changed the title build: migrate log backend to slf4j build: migrate logger to SLF4J APIs + Logback backend Nov 8, 2024
@NiccoMlt NiccoMlt marked this pull request as ready for review November 20, 2024 16:40
@NiccoMlt NiccoMlt self-assigned this Nov 20, 2024
Copy link
Contributor

@hamadodene hamadodene left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @NiccoMlt ... Look good to me...
Please add also a note in the README or in another place for migration from logger to SL4J when user will install this PR.

@NiccoMlt
Copy link
Contributor Author

I don't think that documenting it in the README would be useful, as the logger is an implementation detail that we already are not talking about.

I think we should document it in the changelog when we release it.

What do you think?

@hamadodene
Copy link
Contributor

I don't think that documenting it in the README would be useful, as the logger is an implementation detail that we already are not talking about.

I think we should document it in the changelog when we release it.

What do you think?

Make sense...thanks

@NiccoMlt
Copy link
Contributor Author

To reduce the chance of merge conflicts, consider integrating this after #515 and before #516

@dmercuriali
Copy link
Contributor

@NiccoMlt +1 but a rebase is needed

@NiccoMlt
Copy link
Contributor Author

@dmercuriali thanks, I'll do it later this afternoon

* drop Apache Commons Logging dependency
* add Apache Commons Logging to SLF4J adapter because of other dependencies
* drop JUL backend for SLF4J
* add Logback as backend for SLF4J
� Conflicts:
�	carapace-server/src/main/java/org/carapaceproxy/core/Listeners.java
�	carapace-server/src/main/java/org/carapaceproxy/core/ProxyRequestsManager.java
@NiccoMlt NiccoMlt force-pushed the 288-log-migrate-java-logging-to-sl4j branch from 7bc06c2 to 2e19e4a Compare November 27, 2024 08:37
@NiccoMlt NiccoMlt requested a review from dmercuriali November 27, 2024 08:38
@NiccoMlt
Copy link
Contributor Author

@dmercuriali gave a +1 to the PR yesterday, @hamadodene approved the PR today after the rebase, and the CI passed, so I'm merging it to unlock #516 and hopefully #512

@NiccoMlt NiccoMlt merged commit 88b769f into master Nov 27, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Log > Migrate java logging to SLF4J
3 participants