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

Switch to slf4j #2

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

xxDark
Copy link

@xxDark xxDark commented Jan 4, 2022

Title
I wonder if it is possible to get rid of log4j dependency, so the project can be used more easily.

@rcx
Copy link
Member

rcx commented Jan 4, 2022 via email

@terminalsin
Copy link
Member

Pretty sure there’s more changes required as the logger is used quite literally everywhere

@xxDark
Copy link
Author

xxDark commented Jan 4, 2022

Pretty sure there’s more changes required as the logger is used quite literally everywhere

I'm using this branch locally, and it works fine.
upd: just checked, no log4j references were found

@terminalsin
Copy link
Member

Will check how the output looks during the weekend

@terminalsin terminalsin self-requested a review January 8, 2022 13:18
@terminalsin
Copy link
Member

Info

Current branch has no implementation, causing the logger to fallback to… nothing. It's best to stick with Log4j as of now (Legacy is unaffected and if it happens to be in any case scenario, we'll just upgrade to latest)

@xxDark
Copy link
Author

xxDark commented Jan 8, 2022

It is up to the user to select the implementation. So, to whoever uses this library & however writes an application. That is really the whole point. The library itself should not bundle any implementation.

@terminalsin
Copy link
Member

It is up to the user to select the implementation. So, to whoever uses this library & however writes an application. That is really the whole point. The library itself should not bundle any implementation.

I see. Alright here's what I think is best to do: push usage of SLF4j in the parent context and add a Log4j implementation in the main package. I'll make the changes asap

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.

None yet

3 participants