-
-
Notifications
You must be signed in to change notification settings - Fork 351
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
fix(deps): update dependency org.slf4j:slf4j-api to v2 #4852
Conversation
e6deaff
to
f920a72
Compare
This PR is failing |
f920a72
to
fe23979
Compare
Well, the log tests aren't testing a logging library, they're testing that our implementation of logging is working as expected (with being able to set log levels and whatnot).
Precisely, the backend used for the tests is a... test backend ( |
fe23979
to
5d84b61
Compare
5d84b61
to
14e1af0
Compare
14e1af0
to
f39b39d
Compare
The test logger seems to no longer be picked up by SLF4j, causing the default NOP fallback backend to be used. This, of course, breaks the tests. I didn't find any nice replacement, but apparently logback can do it (or you can do it yourself): |
f39b39d
to
cb48ef6
Compare
Do we plan to solve this PR? I am not sure if the test has enough value to block this PR. Updating the SLF4J bindings has user-facing impact we need to think about though. |
@I-Al-Istannen I agree that the test itself is not enough value to block, but I feel unsure about how this affects consumers of Spoon. I frankly don't quite remember how slf4j works, but I'm guessing that a consumer using a logging implementation incompatible with version 2 would get breakage (much like the test, which is an incompatible implementation). Then again, perhaps we should just always aim to move forward? |
Judging by https://mvnrepository.com/, the share of slf4j 1 users seems to be quite a bit higher. I am not sure what we want here, it would be a breaking change for users. |
cb48ef6
to
c009fa1
Compare
Then I think it's quite evident that we hold off on this upgrade until its adoption is higher. Spoon as a project gains nothing from this upgrade, here it's really all about what the consumers use. |
Anyone against closing this PR because we won't merge this for a long time? |
Renovate Ignore NotificationBecause you closed this PR without merging, Renovate will ignore this update. You will not get PRs for any future If you accidentally closed this PR, or if you changed your mind: rename this PR to get a fresh replacement PR. |
This PR contains the following updates:
1.7.36
->2.0.7
⚠ Dependency Lookup Warnings ⚠
Warnings were logged while processing this repo. Please check the Dependency Dashboard for more information.
Configuration
📅 Schedule: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).
🚦 Automerge: Disabled by config. Please merge this manually once you are satisfied.
♻ Rebasing: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.
🔕 Ignore: Close this PR and you won't be reminded about this update again.
This PR has been generated by Mend Renovate. View repository job log here.