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

chore: bump SLF4j to 2.0.7 (current) #1096

Closed
wants to merge 1 commit into from
Closed

Conversation

milgner
Copy link

@milgner milgner commented Aug 23, 2023

Proposed Changes

Upgrade the dependency on SLF4j to the current 2.0 version. Interfaces are compatible but have been enhanced.

(I was struggling with two different interface versions on my classpath after adding the RabbitMQ client to a project which already uses SLF4J 2.0)

Types of Changes

  • Bugfix (non-breaking change which fixes contributes to issue Bump dependencies #1066)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (correction or otherwise)
  • Cosmetics (whitespace, appearance)

Checklist

  • I have read the CONTRIBUTING.md document
  • I have signed the CA (see https://cla.pivotal.io/sign/rabbitmq)
  • All tests pass locally with my changes
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation (if appropriate)
  • Any dependent changes have been merged and published in related repositories

Further Comments

Did not sign the CA as this is a trivial change that is excluded in CONTRIBUTING.md - please let me know if it's still necessary.

@pivotal-cla
Copy link

@milgner Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@michaelklishin
Copy link
Member

@pivotal-cla this is an obvious fix

@pivotal-cla
Copy link

@milgner This Pull Request contains an obvious fix. Signing the Contributor License Agreement is not necessary.

@pivotal-cla
Copy link

@milgner Thank you for signing the Contributor License Agreement!

@michaelklishin
Copy link
Member

@milgner thank you!

There was an incompatibility at some point in the 2.x series. I no longer remember what it was but it did result in CI failures. Let's see.

@milgner
Copy link
Author

milgner commented Aug 23, 2023

@milgner thank you!

There was an incompatibility at some point in the 2.x series. I no longer remember what it was but it did result in CI failures. Let's see.

Fingers crossed then... I ran the "essential" tests on my local system as described in the README and those passed without a problem. I assume there are more running here - let's hope these run now, too.

@michaelklishin
Copy link
Member

michaelklishin commented Aug 23, 2023

@milgner things seem good with 2.0.7. Let's wait to see if @acogoluegnes has any objections.

@acogoluegnes
Copy link
Contributor

This is a breaking change. SLF4J 1.x and 2.x bindings are not compatible. So an application using a 1.x binding would break as soon as it upgraded to the Java client that uses SLF4J 2.x.

SLF4J 1.x and 2.x are binary-compatible though: applications using the Java client can override SLF4J to 2.x and use the appropriate binding. This is what PerfTest does.

Unless I misunderstood how SLF4J 2.x behaves, we'll have to stick to 1.x to avoid breaking applications, especially considering they can override the dependency.

@milgner
Copy link
Author

milgner commented Aug 23, 2023

When I tried to override the dependency, I got an error about incompatible interfaces 🤔 Unfortunately I don't have it at hand anymore - I'll try and override once more and get back to you afterwards.

@acogoluegnes
Copy link
Contributor

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.

4 participants