-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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] Remove pulsar-broker-common dependency from pulsar-client #17855
[fix] Remove pulsar-broker-common dependency from pulsar-client #17855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Could you open a match PR in your fork to verify it?
I'd suggest applying https://www.mojohaus.org/extra-enforcer-rules/enforceBytecodeVersion.html to prevent future regressions. The Pulsar client modules must be JDK 8 only. This applies to pulsar-common too. |
@lhotari I added the enforcer, it was a great suggestion. Thanks to it, I found out that also Since Pulsar 2.11 will be released using the maven release flag to 17, we absolutely need to apply my latest commit 82beb7b to branch-2.11 before cutting the release
Main class:
I opened a pull against 2.11 #17873 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
/pulsarbot rerun-failure-checks |
Motivation
#17198 added the
pulsar-broker-common
dependency to thepulsar-client
module. This is very bad because in this way user applications will download the pulsar broker internals dependency and they may start using unsupported API.Also this breaks the JDK 8 compatibility for the client. This looks like is no more covered by the CI and I will address it in another pull.
Modifications
doc-not-needed
Fork test runs: nicoloboschi#17